realloc(): invalid next size when reallocating to make space for strcat on char * [duplicate]

Question

I am getting invalid memory error on following code:

printf(" %s\n","FINE 5");
printf("%s LENGTH IS: %d\n","FINE 6",strlen(": "));
buffer = (char *)realloc(buffer, strlen(buffer)* sizeof(char) + (strlen(": ")+1)* sizeof(char));
printf(" %s\n","FINE 7");
strcat(buffer, ": \0");

Output:

FINE 5
FINE 6 LENGTH IS: 2
* glibc detected * ./auto: realloc(): invalid next size: 0x08cd72e0 *** ======= Backtrace: ========= /lib/tls/i686/cmov/libc.so.6(+0x6b591)[0x6dd591]

The point to note here is Fine 7 is never printed. and invalid next size error on every run is at the same location.

Found this relavent

Cassel

What is buffer set to prior to the realloc call? – Saunderson

its a pointer i am reallocating all along, its has about 20 chracters which are being printed correctly – Cassel

Also you should test in case the realloc fails. Unlikely but it does happen – Sayres

I got the same error, when I used realloc after calloc, and I found my stupid error in using calloc which on result returning NULL and I was passing that NULL as input to realloc. I hope you have not such a stupid errors before realloc, also check for null before passing input to realloc. – Maines

Answer 1

This error occurs because some other part of your code has corrupted the heap. We can't tell you what that error is without seeing the rest of the code.

The fact that FINE 7 is not printed tells you that realloc is failing. And that failure must be because buffer is invalid due to a heap corruption earlier in the execution.

---

Orthogonal to your actual problem, sizeof(char) is 1 by definition so it makes sense to remove it from the code.

Bellhop

@ David Heffernan the data in the buffer until Fine 6 is being printed right. – Cassel

If there was no heap corruption, then realloc would succeed. The buffer can print fine but it's the meta data for the memory block that has been corrupted. When memory is allocated there will also be a header block that is used internally by the allocator. That is what is corrupt. Anyway, I've stated my point enough times. – Bellhop

i was using strncpy(arr, arrTemp, strlen(arrtTemp)) i should have used strncpy(arr, arrTemp, strlen(arrtTemp)+1) – Cassel

Answer 2

As David Heffernan points out, your root problem must be a wild pointer elsewhere in your code smashing the heap.

There are several other things worth thinking about in this code snippit, though:

1. No need for sizeof (char) in the new size expression, as sizeof (char) is, by definition, 1.

2. Never assign the return from realloc directly back to the only pointer to the buffer you're reallocating. If realloc returns NULL on an error, you'll lose your pointer to the old buffer, and gain your very own memory leak. You always want to do the appropriate equivalent of:

   footype *p = realloc(oldbuff, newsize);
   if (!p) {
       handle_error();
   } else {
       oldbuff = p;
   }
   

3. In C, void * will automatically be converted to the correct type on assignment, there is no need to cast. Further, by casting, in some cases you won't get helpful error messages when you forget to include the declaration of the function in question.

4. String literals include an implied nul terminator. You wanted to say:

   strcat(buffer, ": ");

On the up side, strcat will stop at the first nul character, so no harm in this case.

Samualsamuel

No buffer overflow by including an extra \0. strcat will proceed to the first \0. It never sees the second one. You can put as many zero terminators in there as you like. – Bellhop

Ach ... of course, strcat will stop at the first nul in the string to copy. I'll correct my answer and note to myself that answering while working is a bad idea! – Samualsamuel

@ 4 already tried that didnt make any difference – Cassel

@jaminator Don't get tricked by all this talk about strcat. The problem is somewhere else in your code where you corrupt the heap. It's not in the code you publish here. – Bellhop

David is right - you've got a wild pointer somewhere else that's smashing your heap. – Samualsamuel

Answer 3

(char *)realloc(buffer, strlen(buffer)* sizeof(char) + (strlen(": ")+1)* sizeof(char));

Should be

(char *)realloc(buffer, (strlen(buffer) + strlen(": ") + 1) * sizeof(char));

should it not? You're math for the length of the string is wrong.

Leibniz

Not so because ac + bc == (a+b)*c – Bellhop

These statements are equivalent. – Saunderson

Irrelevant anyway because char = 1 byte. a * 1 = a – Woll