4

Many people recommend doing the following to reallocate memory:

int *temp=realloc(previousVar,newSize);
if(temp==NULL){
 printf("error\n");
 exit(-1);
}
previousVar=temp;

I understand that, if no new variable was created, the previous pointer would be lost, as NULL would be assigned to it. But what's the point on having all this trouble, if the program will exit in case of failure anyways?

Here's a stack overflow, where the top answer has what I'm referring to: Proper usage of realloc()

  • 1
    But the top answer doesn't have what you show. It shows `free`ing the pointer you `realloc`. By not freeing it, sanitization tools may notice the memory leak. In general, it's good practice to clean up after yourself, especially since more code could be added in the future. – Thomas Jager Mar 06 '21 at 17:41
  • Sure, this was just an example. My question still stands. –  Mar 06 '21 at 17:43
  • 1
    It changes things completely. With what you show, there's no point to using a temporary pointer. With what's in the answer you refer to, there is a point to it. – Thomas Jager Mar 06 '21 at 17:46
  • The program doesn't *have* to exit immediatley. That may be a reasonable default error handling approach, but it *may* be recoverable if the error is propogated up instead. In which case freeing may be appropriate. – StoryTeller - Unslander Monica Mar 06 '21 at 17:49

1 Answers1

0

In the example that you provided it really does not matter if you assign the return value to a temporary or not. In the linked answer, the author frees the original buffer, but one could argue that it is not necessary since the process will exit anyway.

However, consider the following example:


typedef struct
{
    int *values;
    size_t capacity;
    size_t size;
} MyBuffer;

bool append(MyBuffer *buffer, int value)
{
    if (buffer->size == buffer->capacity)
    {
        // No more space, double the buffer.
        size_t new_capacity = buffer->capacity * 2;
        int *temp = realloc(buffer->values, new_capacity);
        if (temp == NULL)
        {
            return false;
        }

        buffer->values = temp;
        buffer->capacity = new_capacity;
    }

    buffer->values[buffer->size] = value;
    buffer->size++;
    return true;
}

The function append tries to add a new value to an already allocated buffer. If the buffer is not large enough, it tries to use realloc to obtain a larger buffer. If realloc fails the function returns false.

If we change the realloc call to set directly buffer->values we will leak memory in case of failure:

        buffer->values = realloc(buffer->values, new_capacity);
        if (buffer->values == NULL)
        {
            // Here the original `buffer->values` is lost and we can never access it again.
            return false;
        }

realloc does not free the original buffer, so in case of failure we no longer have access to it. This is a problem: in the best case scenario it is a memory leak, in the worst case we may have lost some data that we still needed (on top of the memory leak).

So this depends on how you handle realloc failures. If you log an error and exit immediately you don't need to use another variable to store the result. If you report the error up the call chain, or there is some cleanup that you want to do you have to do this. If you take the first approach you need to be careful if you refactor that piece of code to return an error instead of exiting, so there is some merit to always assigning the result to another variable.

icebp
  • 1,608
  • 1
  • 14
  • 24