String.replace() and dynamic memory

Question

I know that Arduino's String.replace function uses realloc().

Is my "replacement" function, which builds a char buffer and then assigns it to the input String, any better in terms of dynamic memory allocation?

I know I should not be using String in the first place, but I am stuck with it for the time being.

This is my function:

void replaceSubstr(String& in, String subin, String subout){

    int s = in.indexOf(subin);

     if(s > -1)
    {
     int a = in.length();
     int b = subout.length();
     int c = subin.length(); 
     int len = (a + (b - c))+1;

    char buff[len];  
    memcpy(buff, in.c_str(), s);
    memcpy(&buff[s], subout.c_str(), b);
    memcpy(&buff[s+b], in.substring(s+c).c_str(), a-(s+c));

     buff[len-1] = '\0';
     in = buff; 
    }
}

Not sure but I think `char buff[len];` may not be supported by some compilers. — Pierre, Mar 31 '16 at 07:35
You may want to read http://stackoverflow.com/questions/1887097/variable-length-arrays-in-c — manuell, Mar 31 '16 at 07:37
@Pierre: This is not part of the C++ standard, only C since C99. — Aconcagua, Mar 31 '16 at 07:37
Arrgh - the link tells VLA in C!!! I let me fool by that, so as talking of C, which is not subject to this question, no I did not... (I certainly would have, if it had been '[...]-in-c++'.) Sorry for that, will be deleting my comments... +1 for manuell's comment... — Aconcagua, Mar 31 '16 at 10:30

Jacek Cz · Accepted Answer · 2016-03-31T08:58:20.670

By the sources

String::String(const char *cstr)
{
    init();
    if (cstr) copy(cstr, strlen(cstr));
}
...
inline void String::init(void)
{
    buffer = NULL;
    capacity = 0;
    len = 0;
}
...
String & String::copy(const char *cstr, unsigned int length)
{
    if (!reserve(length)) {
        invalidate();
        return *this;
    }
    len = length;
    strcpy(buffer, cstr);
    return *this;
}
...
void String::invalidate(void)
{
    if (buffer) free(buffer);
    buffer = NULL;
    capacity = len = 0;
}
...
unsigned char String::reserve(unsigned int size)
{
    if (buffer && capacity >= size) return 1;
    if (changeBuffer(size)) {
        if (len == 0) buffer[0] = 0;
        return 1;
    }
    return 0;
}

Your one line assignment

 in = buff;

Makes all allocation too.

It must be done, original String cannot hold bufferin different memory model, only one 'dynamic - allocated' has sense.

In wide perspective many C memory models (stack, static, allocated by new, allocated by calloc if they are different) must be reduced in real life library - mixing is dangerous. For example stack variable cannot live longer - must be copied to 'allocated'.

You check new possibilities, that's good, but I agree with Aconcagua trust in implementation and not replace original memory model.

Sources: https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/WString.cpp

EDIT: agree with const arguments etc ...

score 0 · Answer 2 · edited May 23 '17 at 11:58

From point of efficiency, you could pass subin and subout as const reference (String const& /*...*/), this avoids copying these two strings.

char buff[len] is not supported in C++, only in C (from C99 on), see e. g. here. You would have to allocate the array on the heap (new char[len]) – unless your compiler supports dynamic arrays on stack as an extension.

Then you could try to reuse the buffer of the in String, this, however, only works if your replacement String is not longer than the one to be replaced (more precisely: the part being longer must fit into the String's buffer being internally allocated to hold the content and possibly being longer than the latter). You would have to move the part of the string after the part to be replaced (e. g. using memmove) before inserting the replacement.

All this requires, however, dealing with internals of the String class (including e. g. adjusting content's size and possibly capacity, if you had to reallocate yourself teh buffer due to being too short) you do not have access to without dirty hacks that will break as soon as the String class changes...

My recommendation: trust in arduino's implementation - you should assume that it does already what you are trying: If the internal buffer is long enough to hold the entire result, it will be used (not needing to move or copy the string within indices 0-s, moving the part from s+c till end appropriately and then copying in the content of subout) and only use reallocate, if the internal buffer is not long enough.

String.replace() and dynamic memory

2 Answers2