Log an array of bytes without fragmenting heap

Question

I am running some code on a STM32 chip which is logging to my uart port.

I am having a hard time finding the proper way to log an array of bytes. I wrote this function:

static void LogBytes(uint8_t *data, uint32_t length)
{
    char message[length*6]={};
    static char byteRepresentation[6];

    for (uint32_t i=0; i<length; i++)
    {
        sprintf(byteRepresentation, "0x%02X ", data[i] & 0xFF);
        strncat(message, byteRepresentation, 6);
    }

    logger.WriteDebug(LoggingCategorySio, message);
}

But when I run this, I eventually get hard faults (which are a pain to troubleshoot, but I suspect it's caused by heap corruption/fragementation).

I think the problem is caused by strncat (the comment to this answer warns about some problems I might be experiencing). I also read to stay away from malloc / free as much as possible. So I am trying to do as much as I can on the stack.

Am I missing some stupid bug in this code? Or should it be done completely different?

From the comment:

"there are other, quicker and more efficient alternatives (such as strcpy() or memmove()) that could be used instead. Pretty much whatever the question of 'what should I use', strncat() is not the answer"

Is it indeed better to use strcpy / memmove? Or do they in fact rely more on malloc / free and thus should I avoid that even more?

Since the length of one token is fixed in this case, it looks better to specify the place to construct the string directly than using funtions like `strcpy` or `memmove`, like `sprintf(message + 6 * i, "0x%02X ", data[i] & 0xFF);` Also note that you have to allocate one more element for the terminating null-character. — MikeCAT, Jun 15 '21 at 19:25
Each loop is creating a `byteRepresentation[]` string length `5` bytes (and terminator) so there should not be any overflow, which would be on the stack not the heap. Perhaps some there is some other cause of heap fragmentation. — Weather Vane, Jun 15 '21 at 19:34
@MikeCAT tried your suggestion, but I guess something else is causing it. I can control the maximum amount of bytes my reading thread takes in. If I reduce it (thus decrease the stack size significantly) the problem seems to be fixed. I will have to dig further. Thx for the better implementation, I'll stick with this anyway. — bas, Jun 15 '21 at 19:50
Oops, `"0x%02X "` actually produces 5-character strings like `0xAA `, so there actually no need to allocate an extra space. Sorry for mistake. — MikeCAT, Jun 15 '21 at 19:55
Yeah figured that when I copy pasted your suggestion and I was printing 1 byte instead of a whole bunch. I miscounted myself, my initial code caused the confusion. — bas, Jun 15 '21 at 20:02

score 3 · Accepted Answer · answered Jun 15 '21 at 19:36

3

If the problem did end up being from heap overuse (from strncat), then you could try out this implementation that uses the return from sprintf to append to the string as your building it.

static void LogBytes(uint8_t *data, uint32_t length)
{
    char message[length*6];
    memset(message, 0, length*6);
    size_t message_offset = 0;

    for (uint32_t i=0; i<length; i++)
    {   
        int ret = sprintf(message + message_offset, "0x%02X ", data[i] & 0xFF);
        if (ret > 0){
            message_offset += ret;
        } else {
            //TODO react however you want to sprintf error
            break;
        }   
    }   

    logger.WriteDebug(LoggingCategorySio, message);
}

However, there could also be problems from overflowing the stack. If that's the problem, then this variable length array char message[length*6]; might be the culprit for hardfaulting when printing super large arrays.

answered Jun 15 '21 at 19:36

GandhiGandhi

1,029
6
10

1

Little things: 1) Nice alterative to `memset(message, 0, length*6);`: `memset(message, 0, sizeof message);`. 2) With `"0x%02X "`, could use `length*5 + 1`. – chux - Reinstate Monica Jun 15 '21 at 19:44
Thx for the answer. Using your `sprintf` which was suggested by MikeCAT too. The memset doesn't add anything over `char message[size] = {};` right? Anyway, I think your comment on super large arrays is indeed the cause. When I reduce the maximum amount of bytes read each cycle the problem dissappears. I have asserts on stackhighwatermark (freertos). They are all satisfied. I'll dig a bit deeper. But this really helped – bas Jun 15 '21 at 19:55
If I may, one side question: does `strncat` or `sprintf` malloc memory under the hoods, and should I be concerned with that (e.g. does it fragment my heap?). I see that a library I use (lwip) mallocs a big deal as well. Since that is meant to run on microcontrollers I am a bit lost on "never use malloc / free". – bas Jun 15 '21 at 20:00
@bas `char message[size] = {};` --> "error: variable-sized object may not be initialized". – chux - Reinstate Monica Jun 15 '21 at 20:16
`char message[(length*5)+1]={};` this compiles fine though – bas Jun 15 '21 at 20:30
About the `memset` line, With gcc 8, I was getting a compiler error on this line: `char message[(length*5)+1]={};` but if that compiles for you, I like that solution without `memset` better :) – GandhiGandhi Jun 15 '21 at 20:33

Log an array of bytes without fragmenting heap

1 Answers1