CODEWITHSUNDEEP
c
xambo
xambo

Reputation: 419

Memory corruption in the last byte

I have a function that returns a pointer to a structure as the following : 

//header file
typedef struct {
        unsigned char *buffer;
        uint8_t       len;
} T_ABC_PACKET

in the main file, I create a pointer to a function and tries to print this

T_ABC_PACKET *pct = NULL;
pct = function_that_return_the_packet;
printf("value of packet is %s \n", pct->buffer);

the result is always consistent in the printing function. I expect the buffer to have an 8 byte, and the last byte is always a corrupted memory. value is 10000357`�2U

but if I print the buffer inside the function :

T_ABC_PACKET* function_that_return_the_packet {

T_ABC_PACKET *pct = NULL;
char string_temp[80];
//some more initialization...
pct->buffer = (unsigned char *)string_temp;
pct->len = 5;
printf("value of packet is %s \n", pct->buffer);
return pct;
}

the value printed in the function is 10000357f. Only the last character is corrupted. This always provide a consistent value, no many times I run the program, only the last character is corrupted in the caller of the function. I understand one possible case is memory leak, but I tried to check carefully and I can not find any leak. How do I get the pct->buffer to have everything correctly?

Upvotes: 0

Views: 221

Answers (3)

Shafik Yaghmour
Shafik Yaghmour

Reputation: 158469

It looks like you are returning a pointer to a local variable which is undefined behavior, string_temp is local to function_that_return_the_packet and will not exist after you exit that function.

As Daniel suggested using strdup is probably the simplest way to fix the problem:

pct->buffer = strdup(string_temp);

Just make sure you check that it did not fail. You could of course also use malloc and then strcpy.

Upvotes: 5

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726539

Once you fix the undefined behavior of returning a pointer to local (see Shafik Yaghmour answer) you still have undefined behavior: it appears that the buffer is not null-ternminated, so %s format specifier reads past it, and stops only when it finds an unrelated \0.

If you know that the buffer's length cannot exceed eight, you can copy its content up to pct->len into a char buffer, theninsert a terminator at the end:

char tmpBuf[9]; // max length is 8, plus one for null ternminator
memcpy(tmpBuf, pct->buffer, pct->len);
tmpBuf[pct->len] = '\0';
printf("value of packet is %s \n", tmpBuf);

Upvotes: 1

Daniel Mošmondor
Daniel Mošmondor

Reputation: 19956

This is the source of the problem:

pct->buffer = (unsigned char *)string_temp;

'string_temp' is allocated on the stack. When function returns, it is destroyed somewhere later, or not, as in your case, except last byte.

You should:

Use strdup() instead of assignment in that line.

When you are done with whole structure, use free() to release that string before releasing whole structure.

Upvotes: 0

Related Questions