heap corruption when using free()

Question

I am new to dynamic allocation in c and I am getting heap corruption error at the call of the free() function.

The whole code is supposed to simulate the reallocation function realloc()and it works fine until the end. I ran the code multiple times in debugger mode step by step and the error appears at the end. If someone could help me I would be very grateful.

#include <stdlib.h>
#include <stdio.h>


void realocare(int **v, int n,int m)
{
    int *aux;
    unsigned int i;

    aux = (int*)malloc(m*sizeof(int));

    for (i = 0; i < m; i++)
        aux[i] = v[i];

    *v = (int*)malloc(n*sizeof(int));

    for (i = 0; i < m; i++)
        v[i] = aux[i];
    free(aux);
}

void afisare(int *v, int n,int i)
{
    for (i = 0; i < n; i++)
        printf_s("%d ", v[i]);

        printf_s("\n");
}

int main()
{
    int *v;
    unsigned int n,i,m;

    scanf_s("%u", &n);

    v = (int*)malloc(n*sizeof(int));

    for (i = 0; i < n; i++)
        scanf_s("%d", &v[i]);

    m = n;
    printf("%d", m);
    afisare(v, n, i);
    n++;
    realocare(&v, n,m);
    v[n - 1] = 9000;
    afisare(v, n, i);

    free(v);
    return 0;
}

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Dec 02 '16 at 15:20
at `realocare` : `v = (int*)malloc(n*sizeof(int));` : It does not change the variable on the caller side. — BLUEPIXY, Dec 02 '16 at 15:27
Even if `realocare()` didn't risk writing out of bounds (described in @alk's answer), it doesn't really do anything except leak memory. — Dmitri, Dec 02 '16 at 15:28
OT: All those casts to `malloc()` are useless in C. Just drop them. — alk, Dec 02 '16 at 15:29
`*v = (int*)malloc(n*sizeof(int));` is a memory leak. You are losing the old `*v` pointer without freeing it first. You want a `free(*v)` before this line. — Kaz, Dec 02 '16 at 16:00

alk · Accepted Answer · 2016-12-02T16:08:10.277

You allocate to v n elements

v = ... malloc(n*sizeof(int));

and assign m

for (i = 0; i < m; i++)
    v[i] = aux[i];

For the case that m is larger then n: Doing so writes to invalid memory, and with this invokes undefined behaviour, so anything can happen from this moment on.

In your particulate case most likely this messes up internal memory management structures, causing the failure a later call to free().

Changes done to a variable which had been passed to a function are not reflected by the caller, as the function in C always and ever just receives a copy of what the caller passed down.

So, for example the new value you assigned to v stays unknown to the caller of realocare().

You can fix this by adjusting the code as follows:

void realocare(int **ppv, int n, int m)  //reallocation simulation function
{
  int *aux;
  unsigned int i;

  aux = malloc(m*sizeof(int));

  for (i = 0; i < m; i++)
    aux[i] = (*ppv)[i];

  free(*ppv); // Free what you had, to not leak this memory.

  *ppv = malloc(n*sizeof(int));

  for (i = 0; i < m; i++)
    (*ppv)[i] = aux[i];

  free(aux);
}

And call it like this:

realocare(&v, n, m);

Your code uses two calls to malloc() and two calls to free(). This is inefficient.

Look at the following (adding some other not just cosmetic changes):

void realocare(int **ppv, size_t n, size_t m)  // no need for negative sizes ...
{
  int * aux = malloc(n * sizeof *aux);
  size_t i = 0; // no need for negative counters ...

  for (;i < m; ++i)
  {
    aux[i] = (*ppv)[i];
  }

  free(*ppv); 

  *ppv = aux;
}

Just one malloc and one free... :-)

And for completeness a robust version:

int realocare(int **ppv, size_t n, size_t m)  
{
  int result = -1; // be pessimistic

  if (NULL == ppv)
  {
    errno = EINVAL;
  }
  else
  {
    int * aux = malloc(n * sizeof *aux);

    if (NULL != aux) 
    {
      size_t i = 0; 

      for (;i < m; ++i)
      {
        aux[i] = (*ppv)[i];
      }

      free(*ppv); 

      *ppv = aux;

      result = 0;  // return success! 
    }
  }

  return result;
}

Call it like this:

#include <stdlib.h>
#include <stdio.h>
#include <errno.h>  // for errno

...

int main(void)
{
  ...

  if (-1 == realocare(&v, n, m))
  {
    perror("realocare() failed");
    exit(EXIT_FAILURE);
  }

thanks but im not sure if i figured it out, i made the following changes but it still doesn't work: 'void realocare(int ***v, int n,int m)', '*v = (int*)malloc(n*sizeof(int)); ' realocare(&v, n,m);' — rednefed, Dec 02 '16 at 15:43
@rednefed: You are welcome. For fun I added a stripped own version of your code. — alk, Dec 02 '16 at 15:54

heap corruption when using free()

1 Answers1