Search code examples
cmemory-managementpointersbufferrealloc

Problem with realloc() in C. Always hangs but compiles fine


I'm having some trouble with a program that is intended to be a String buffer, specifically this function is intended to reset the buffer with the string cstr. If cstr is null then the content needs to be reset to an empty char '\0'. It always hangs at the second set of realloc where it's resizing buf->contents I have no clue why that is. Any help would be awesome.

The struct:

typedef struct strbuf {
     char   *contents;
     size_t  length;  
} StringBuffer;

It is called from

strbuf_reset(sb, NULL)

Here is the strbuf_reset function that is having the problem.

StringBuffer *strbuf_reset(StringBuffer *buf, const char *cstr)
{
if(buf == NULL)
    return NULL;

StringBuffer *tempBuf = NULL ;

if(cstr == NULL)
    tempBuf = (StringBuffer*)realloc(buf,sizeof(StringBuffer) + sizeof(char));
else
    tempBuf = (StringBuffer*)realloc(buf,sizeof(buf) + strlen(cstr)*sizeof(char));

if(tempBuf == NULL)
    return NULL;

if(cstr == NULL)
    tempBuf->contents = (char*)realloc(buf->contents,sizeof(char));
else
    tempBuf->contents = (char*)realloc(buf->contents,(sizeof(buf->contents) + strlen(cstr)*sizeof(char) + 1));

if(tempBuf->contents == NULL){
    free(tempBuf);
    return NULL;
}
buf = tempBuf;

if(cstr == NULL)
   buf->contents = '\0';
else
   strcat(buf->contents,cstr);

buf->length = strlen(buf->contents);    

return buf;
 }

With what I believe to be the suggested changes...

StringBuffer *strbuf_reset(StringBuffer *buf, const char *cstr)
{
if(buf == NULL)
    return NULL;

StringBuffer *tempBuf = NULL ;

if(cstr == NULL)
    tempBuf = (StringBuffer*)realloc(buf,sizeof(StringBuffer) + sizeof(char) + 10);
else
    tempBuf = (StringBuffer*)realloc(buf,sizeof(buf) + strlen(cstr)*sizeof(char)+ 1);

if(tempBuf != NULL)
    buf = tempBuf;
else
    return NULL;    

if(cstr == NULL)
    tempBuf->contents = (StringBuffer*)realloc(buf->contents,sizeof(StringBuffer) + sizeof(char) + 10);
else
    tempBuf->contents = (StringBuffer*)realloc(buf->contents,sizeof(buf) + strlen(cstr)*sizeof(char)+ 1);

if(tempBuf != NULL)
    buf->contents = tempBuf->contents;
else
    return NULL;

if(cstr == NULL)
   buf->contents = '\0';
else
   strcat(buf->contents,cstr);

buf->length = strlen(buf->contents);    

return buf;
 }

Solution

  • Since you are going to overwrite StringBuffer's contents, using realloc makes no sense. It will not save you any memory allocation but instead will copy old data you intent to overwrite anyway. Use normal malloc and free.

    With your original structure

    typedef struct strbuf {
      char   *contents;
      size_t  length;  
    } StringBuffer;
    

    strbuf_reset sets buf to cstr. On success returns buf, on failure NULL.

    StringBuffer *strbuf_reset(StringBuffer *buf, const char *cstr)
    {
      if (!buf)  return NULL;      
      if (!cstr)  cstr = "";
    
      size_t len = strlen(cstr);
      if (len > buf->length) {
        char *new_contents = malloc(len + 1);
        if (!new_contents)  return NULL;
        free(buf->contents);
        buf->contents = new_contents;
      }
      memcpy(buf->contents, cstr, len + 1);
      buf->length = len;
    
      return buf;
    }