realloc() query

[Neo]

I am not sure if this code I saw is correct but it sure looks like buggy to me (not sure if its because I'm not yet fully awake)

Code: Select all

if( freeSize < strlen(node_p) )
{
   if( (dispBuf = (char*)realloc(dispBuf, totalBufSize+BufSize) )
       == NULL )
   {  //realloc has failed, make sure no overwrite into stack!
      strncat (dispBuf, node_p, freeSize);
   }
   else
   {
      strncat (dispBuf, node_p, BufSize);
      totalBufSize = totalBufSize+BufSize;
   }
}
else
{
   strcat (dispBuf, node_p);
}

[Solar]

Yeee-harrr!

I don't know what that code is intended to accomplish, but I sure hope the coder was aware that what ends up in dispBuf is not delimited by zero bytes. I also don't see freeSize being adjusted after the strncat / strcat calls, but that might be happening elsewhere. What's really juicy is the realloc() part though:

Code: Select all

if( (dispBuf = (char*)realloc(dispBuf, totalBufSize+BufSize) ) == NULL )
{  //realloc has failed, make sure no overwrite into stack!
   strncat (dispBuf, node_p, freeSize);
}

If you want to handle out-of-memory situations correctly, never allocate the return code of realloc() to what you pass to it as first parameter: realloc() returns NULL on out-of-memory, but its first parameter is still a pointer to very valid allocated memory you could never free() again if you overwrote the pointer with NULL.

But this code will never get that far because strncat() will copy freeSize bytes of node_p to NULL, and you know what that means.

Then again, most (paging) OS's die violently on out-of-memory without any malloc() ever returning NULL, anyway. (malloc() always succeeds because the pages it allocates aren't actually mapped until much later when they're actually used, which means the PF handler dies long after the actual malloc() succeeds, a weakness shared by many operating systems AFAIK.)

Code: Select all

else
{
   strncat (dispBuf, node_p, BufSize);
   totalBufSize = totalBufSize+BufSize;
}

This merely truncates node_p to BufSize bytes (if sizeof( node_p ) + 1 > BufSize), which might or might not be what you want. Oh, and I prefer the += operator in cases like this, but that's just code candy.

[Neo]

Thanks for confirming my guess Solar.
I was pretty sure the code was screwed up but was not sure about how realloc() worked.

So realloc() does not free the previously malloc()'ed memory if it fails?
Maybe it should do that, anyway.......

[Solar]

Nononono... you want realloc() to throw away your document when it grows too large? Hey, we aren't Microsoft...

[rich_m]

So then is the following code correct

Code: Select all

  if( (newDispBuf = (char*)realloc(dispBuf, totalBufSize+BufSize) )
      == NULL )
  {  //realloc has failed, make sure no overwrite into stack!
      strncat (dispBuf, node_p, freeSize);
  }
  else
  {
      free(dispBuf); // this is needed isn't it?
      dispBuf= newDispBuf;
      strncat (dispBuf, node_p, BufSize);
      totalBufSize = totalBufSize+BufSize;
  }

Is the free() of dispBuf needed?

[Solar]

Is the free() of dispBuf needed?

No, it is not.

[Pype.Clicker]

reminds me of why i tell my students to stay away from realloc() ;D

[Solar]

Like everything in ASM, C, or C++: Use only if you know what you're doing. Thinking you know what you're doing is a #1 recipe for desaster... erm, segfaulting.

[Candy]

Thinking you know what you're doing is a #1 recipe for desaster...

Disaster?

[Solar]

Might be. Hey, I can't be perfect in everything.

[Neo]

Is the free() of dispBuf needed?

No, it is not.

So dispBuf is freed by realloc() if it gets a new memory location?

[Solar]

Short answer: yes.

Long answer: ISO/IEC 9899:1999 (E), 7.20.3.4 The [tt]realloc[/tt] function, emphasis mine:

Synopsis

[tt]#include <stdlib.h>
void *realloc(void *ptr, size_t size);[/tt]

Description

The [tt]realloc[/tt] function deallocates the old object pointed to by [tt]ptr[/tt] and returns a pointer to a new object that has the size specified by [tt]size[/tt]. The contents of the new object shall be the same as that of the old object prior to deallocation, up to the lesser of the new and old sizes. Any bytes in the new object beyond the size of the old object have indeterminate values.

If [tt]ptr[/tt] is a null pointer, the [tt]realloc[/tt] function behaves like the [tt]malloc[/tt] function for the specified size. Otherwise, if [tt]ptr[/tt] does not match a pointer earlier returned by the [tt]calloc[/tt], [tt]malloc[/tt], or [tt]realloc[/tt] function, of if the space has been deallocated by a call to the [tt]free[/tt] or [tt]realloc[/tt] function, the behavior is undefined. If memory for the new object cannot be allocated, the old object is not deallocated and its value is unchanged.

Returns

The [tt]realloc[/tt] function returns a pointer to the new object (which may have the same value as a pointer to the old object), or a null pointer if the new object could not be allocated.

[rich_m]

Is the following code incorrect in any way

Code: Select all

  if( (newDispBuf = (char*)realloc(dispBuf, newBufSize) )== NULL )
  {
      strncat (dispBuf, node_p, freeSize);
  }
  else
  {
      dispBuf= newDispBuf;
      strncat (dispBuf, node_p, BufSize);
  }

Does realloc free the existing memory even if it returns NULL/fails?

Also where can i get that document, googling didnt seem to help.

[Solar]

Is the following code incorrect in any way

Looks good.

Does realloc free the existing memory even if it returns NULL/fails?

"If memory for the new object cannot be allocated, the old object is not deallocated and its value is unchanged."

Ergo, no it does not.

Also where can i get that document, googling didnt seem to help.

It's an ISO standard document, so you can get it at the ISO website. However, any good C language reference book, or visiting the Dinkumware online reference, or typing "man {function}" should get you about as far.