Heap corruption while free memory

Question

I have function that allocates memory for char* c variable and later frees it. Got error while debug:

---------------------------
Microsoft Visual C++ Runtime Library
---------------------------
Debug Error!

HEAP CORRUPTION DETECTED: after Normal block (#95) at 0x0130BE00.
CRT detected that the application wrote to memory after end of heap buffer.

What I do wrong?

void logHex(char* value, int len, int level)
{
    if (LogHnd)
    {
        char* c = (char*)calloc(len * 3, 1);
        for (int i = 0; i < len; i++)
        {
            sprintf(c, "%s%02X ",c,value[i]);
        }
        LogHnd(c, level);
        free(c);
    }
}

`sprintf(c, "%s%02X ",c,value[i]);` - Why do you write `c` as a string (`%s`) into itself ? — wohlstad, Dec 26 '22 at 10:08
You need to allocate an extra character for the null terminator — Alan Birtles, Dec 26 '22 at 10:09
A correct solution using `sprintf` uses a temporary string `char t[4]; sprintf(t, "%02X ", value[i]); strcat(c, t);` Assuming 0 <= value[i] <= 255. A more efficient version would calculate the offset and use `strcpy` instead of `strcat`. — john, Dec 26 '22 at 10:18
@vico Apparently it's not allowed (see answer below). Even if allowed it would be inefficient. Another possible error is a negative value in your array. — john, Dec 26 '22 at 10:20
@vico No format arguments might overlap with the destination buffer, so you can't use `c` as both the destination and the source for a `%s` format specifier. — Some programmer dude, Dec 26 '22 at 10:20
And I really recommend you invest in [some good C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) to learn C++ properly. Any time you must use C-style casting (like you do for the result of `calloc`) should be taken as a sign that you're doing something wrong. — Some programmer dude, Dec 26 '22 at 10:23
char* calloc, sprintf and free Your code is "C" not "C++". In C++ I'd expect things like std::string, std::make_unique, std::format and std::cout — Pepijn Kramer, Dec 26 '22 at 10:33
@vico Your array is of `char`, `char` maybe (and usually is) a signed type. So the values in your array go from -128 to 127. But I'm guessing you want to print values from 0 to 255. So negative values are going to print incorrectly. To fix this you should cast to `unsigned char`, i.e. `(unsigned char)value[i]`. — john, Dec 26 '22 at 14:04

Some programmer dude · Answer 1 · 2022-12-26T10:25:40.220

Using the same pointer as both source and destination in sprintf leads to undefined behavior.

Use std::string for all your string needs in C++, and then you can use the plain + operator to append strings.

To format strings use std::ostringstream, the format library or std::format (if your compiler is new enough).

Using std::ostringstream it could be something like this:

std::string c;

for (int i = 0; i < len; ++i)
{
    c += (std::ostringstream{} << std::hex << std::setw(2) << std::setfill('0') << static_cast<unsigned>(value[i])).str();
}

Using std::format it could be something like:

std::string c;
for (int i = 0; i < len; ++i)
{
    c += std::format("{:02x}", static_cast<unsigned>(value[i]));
}

With the format library it would be the same, but with the fmt namespace instead of std.

Note that I cast the char value in value[i] to an unsigned integer, because it's implementation defined if char is signed or unsigned. If char is signed, then values above 0x7f would be considered negative, and lead to sign extension which would add a lot of leading f hexadecimal digits to the value.

Heap corruption while free memory

1 Answers1