1

Because I wanted to support both ESP32 and AVR based boards, I had to change from the things like std::string to an alternate method. I wanted to avoid the String class because I've heard that's a bit of a mess memory wise. I decided to use char arrays and null terminated strings, and make sure to free them all myself.

From my app, over Bluetooth, I recieve data in the format <id>:<data>, where id is a two digit number (i.e. 01). Once I receive data, I pass it on to this function, which does some processing:

void CDataUtils::process(char *data) {
    char *split = strtok(data, ":");
    m_id = static_cast<ActionID>(atoi(split));

    uint16_t len = strlen(data) - (strlen(split) + 1);
    char *d = new char[len + 1];

    strcpy(d, split + strlen(split) + 1);

    delete[] split;
    //......
    delete[] d;
}

I split the string with strtok, and use both side so I store that in a variable called split. I take the first section which is the id and cast it to an enum but that is irrelevant for this. I then calculate the length of the data after the colon and create a char array of that size (plus 1).

I then copy the data that comes after the colon into d. Once that is done, because I no longer need split, I free it with delete[]. This causes my ESP to crash though

CORRUPT HEAP: Bad tail at 0x3ffdc795. Expected 0xbaad5678 got 0xbaad5600
abort() was called at PC 0x40081d91 on core 1

ELF file SHA256: 0000000000000000

Backtrace: 0x4008e254:0x3ffc71f0 0x4008e4d1:0x3ffc7210 0x40081d91:0x3ffc7230 0x40081ebd:0x3ffc7260 0x400dd653:0x3ffc7280 0x400d8c65:0x3ffc7540 0x400d8bf4:0x3ffc7590 0x400931b5:0x3ffc75c0 0x400817f6:0x3ffc75e0 0x40081cd5:0x3ffc7600 0x4000bec7:0x3ffc7620 0x40104059:0x3ffc7640 0x400d1fa7:0x3ffc7660 0x400d1e9c:0x3ffc7680 0x400d1b0e:0x3ffc76a0 0x400d32e5:0x3ffc76c0 0x4008ffda:0x3ffc76e0

According to the first link when googling for CORRUPT HEAP: Bad tail, this is usually a sign of writing data outside of a buffer. Surely if that was the case here, the error would occur at the strcpy line, not at the delete line? What confuses me is I have this little utility function that deletes and reallocates a new array:

char *_realloc_and_null_array(char *arr, uint16_t size)
{
    delete[] arr;         //Free array
    arr = new char[size]; //Create new one of specified length

    memset(arr, 0x00, size); //Fill with null

    return arr;
}

I used to use this in the process function above, and I did not get any errors with it even though I am freeing an array with delete[].

What have I done wrong?

EDIT: I'll reply to the comments here because there's a lot. I appreciate the info on strtok and how I don't need to work out the length of the data, or copy the data to any buffer.

The reason I was trying to delete the pointer to the split was because I didn't realise it was a pointer. I'm not sure how I thought it could be a string but I did/

In terms on the _realloc_and_null_array function it was purely to remove duplicate code. I got the snippet from an SO post but I can't remember the one. When I create a program (in any language) I don't like to just create variables all willy-nilly. In this instance, if I have an array d, I can use that for something, free it, create a new one and fill it with different data. This saves having another unnecessary array. I re-use arrays a fair bit so it allows me to just create a new array with a different length and get rid of the old one (I don't do it a stupid amount as to where the program is hard to understand. I'll use the function at max once in some functions).

Maybe it's a dumb thing to do but it worked for me so I stuck with it.

10
  • 1
    String is bad, because it uses dynamic allocation, just as you do. That leads to memory fragmentation. And in the cases, where this doesn't happen, you just can use static allocation, without loosing functionality. Though it is a bit unclear to me, why you are trying to delete split instead of data (since split will just be a pointer to the start of the first token inside data, which is just the start of data). Or aren't you intending on deleting data?
    – chrisl
    Jan 10 at 15:19
  • And what exactly is the purpose of the _realloc_and_null_array() function? Why would you want to first delete the array and then recreate it again, when you fill the memory with zeros afterwards anyway? What problem are you trying to solve with that?
    – chrisl
    Jan 10 at 15:20
  • 2
    strtok returns a pointer to the first token in a list of tokens separated by ':'. It does not create a new array. There is no need to delete the array pointed by split. split only points into the data array not to its own array. But strtok is destructive. It changes the content of the data string as it writes \0 instead of ':' in your case. You can call d=strtok(NULL, ":") a second time and you get the next token. No need to compute the length or d in such a complecated way. Jan 10 at 15:27
  • majenko.co.uk/blog/splitting-text-c
    – Majenko
    Jan 10 at 15:32
  • This has nothing to do with the problem itself, but you can new char[size]() rather than new char[size] followed by your memset to '\0'. The addition of the parenthesis will take care of that for you. That said, a large part of why strtok is destructive to its input so that you do not necessarily even need to copy the data let alone dynamically allocate to process it; frequently you don't.
    – timemage
    Jan 10 at 16:17
0

Ok, so you have some misconceptions here:

  • strtok() will not create a new c-string/character array. It returns a pointer, which points to the place in data, where your current token begins. So dong delete[] on split does not make much sense. Also note, that strtok() will alter the c-string, that you give to it. So data will be altered, after the function executed. strtok() inserts a null character \0 at the place of the delimiter (: in this case. That way c-string functions can be provided with the pointer, that strtok()` returns, and will only read until the end of the token (since they look of the null character as end of string).

  • The String class is bad on Arduino (also on the ESP, but not in the same urgency), because they have very few memory for variables. The String class allocates its memory dynamically, everytime, that you do concat, assign or change the size of the string. That means, it creates and destroys arrays with different sizes rather often, which will lead to memory fragmentation. You will get small holes in your memory, which are too small for new allocations. With the time you will get more and more holes, until your memory is full. Then the Arduino crashes. Since Arduinos often have very few memory (and don't have an OS, that manages the RAM), that happens rather fast. The ESP has more memory, so it doesn't happen that fast, but the problem is still the same.

    It is the same situation with every dynamic memory allocation. In the rare cases, where you don't get memory fragmentation, you can as easy use a local variable (managed automatically on the stack for you).

  • In this instance, if I have an array d, I can use that for something, free it, create a new one and fill it with different data. This saves having another unnecessary array.

    Actually: No, not really. The array d is created somewhere at the start of the process() function and deleted at the end of it. Just like a local variable (statically allocated). The only benefit here is, that d can have a variable size (not know at compile time). Though that can also create problems with the dynamic allocation, as described above (depends on if you are doing dynamic allocation in between of the allocation and deletion).

  • The _realloc_and_null_array() function makes only sense, when the size is different from the size of the provided array. But then again you can run into the issue described above.

  • I don't like to just create variables all willy-nilly.

    What do you mean with "willy-nilly"? You are only creating variables, that you really need. But that doesn't mean, that these variables cannot be statically allocated. A common misconception, that programmers coming from bigger platforms (like PCs) often have, is that you should keep the memory footprint as low as possible, thus using dynamic allocation to free resources, that aren't currently needed. On a PC the freed memory can be used by other programs running on it. But on an Arduino there is only one program running. Memory, that you don't use, will just be doing nothing. So keeping the memory footprint is important, when you are actually running out of memory for your program, but not important, when you still have enough memory. As dynamic memory allocation comes with big caveats on these platforms, it is mostly not really worth it.

All in all: Dynamic memory allocation comes with huge caveats. You cannot just use it like on a PC. If you still can use it depends on your code and your requirements (like on code runtime, before it gets unstable). There are situations, where you can get away with it, and others, where you don't.

4
  • Thanks for the help. I just want to clarify then because I don't know the length of the data at compile time, I went with dynamic allocation, like creating an array the length of the data plus 1. I specifically make sure to free that at the end though because that's what you need to do. So am I still causing memory fragmentation with my program or is it that I'm just implementing it in a bit of a long winded way? Jan 10 at 19:36
  • Also, I mean "willy-nilly" as in when I was calculating the length of the data I could have very easily created a variable for the length of the string, one for the size of the size of the colon and so on. But that's pointless. So in terms of this post, I could create one array for the id, and one for the data but if I can just create one and replace the data to me that makes more sense. Of course, as I've learnt, I don't need any array with strtok so that's not a great example. Jan 10 at 19:41
  • Aside: Strings don't always cause a program to spontaneously combust. On their own a few String filenames or SSIDs or web responses don't hurt anything. It's basically only when you process data (split, concat, substring) using String methods that you run into issues.
    – dandavis
    Jan 12 at 4:18
  • I could definitely limit the number of operations I need to do on strings by modifying the data I send from my app into a more useable manner. The worst one is gradients in which I just send over the raw CSS string so it requires a couple regex matches, some splits and so on, so I avoided String because of that. Jan 12 at 17:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.