17

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

Community
  • 1
  • 1
PUG
  • 4,301
  • 13
  • 73
  • 115

3 Answers3

21

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.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • @ David Heffernan the data in the buffer until `Fine 6` is being printed right. – PUG Dec 08 '11 at 20:18
  • 2
    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. – David Heffernan Dec 08 '11 at 20:21
  • 1
    i was using strncpy(arr, arrTemp, strlen(arrtTemp)) i should have used strncpy(arr, arrTemp, strlen(arrtTemp)+1) – PUG Dec 09 '11 at 15:24
9

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.

Greg Jandl
  • 831
  • 9
  • 12
  • 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. – David Heffernan Dec 08 '11 at 20:12
  • 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! – Greg Jandl Dec 08 '11 at 20:14
  • @ 4 already tried that didnt make any difference – PUG Dec 08 '11 at 20:15
  • @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. – David Heffernan Dec 08 '11 at 20:16
  • 1
    David is right - you've got a wild pointer somewhere else that's smashing your heap. – Greg Jandl Dec 08 '11 at 20:24
0

(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.

Grambot
  • 4,370
  • 5
  • 28
  • 43