realloc misuse

[KBriggs]

Hey,

I have a function that uses a 2D array of doubles that needs to be reallocated every now and again as the input size changes with time. This is the function. The first time this function is called, particleMatrix is NULL (ei, I use it like malloc on the first call)

int readTimeStep(FILE *input, double **particleMatrix)
{
  int i,N;
  double time;

 fscanf(input,"%d",&N);
 fscanf(input,"%*s %*s %*d %*s %*s %*s %*s %*s %*s %*s %*s %lf",&time);
 
  /// 
  particleMatrix = (double**) realloc(particleMatrix, N * sizeof(double*));
  if (particleMatrix == NULL)
  {
    printf("error allocating memory for particleMatrix");
    abort();
  }
  for (i = 0; i < N; i++)
  {
    particleMatrix[i] = (double *) realloc(particleMatrix[i], 4 * (sizeof(double)));
    if (particleMatrix[i] == NULL)
    {
      printf("error allocating memory for particleMatrix[i]");
      abort();
    }
  }  
  ///
  for (i = 0; i < N && fscanf(input,"%*s %lf %lf %lf %lf",&particleMatrix[i][0],&particleMatrix[i][1],&particleMatrix[i][2],&particleMatrix[i][3]) != EOF; i++)
  {
    ;
  }

 return time; //reallocate matrix for each N during loop
}

When I run this, I get:

*** glibc detected *** ./chargestate: realloc(): invalid pointer: 0x0000000000400f73 ***

What am I doing wrong?

[laserlight]

One thing that comes to mind is that you are assigning to the parameter in the function. If you intend the changes to the parameter to be reflected in the caller, you should pass a pointer to it. Have you considered defining a struct for particleMatrix?

[KBriggs]

particleMatrix is declared in the caller - the realloc in this function should be reflected in the caller, no? Or will realloc cause the caller to lose track of the pointer to the start of particleMatrix?

I have never used a struct like that before. Do you mean something like a linked list?

EDIT: putting this function inside the caller and getting rid of the call results in the same error

[laserlight]

particleMatrix is declared in the caller - the realloc in this function should be reflected in the caller, no? Or will realloc cause the caller to lose track of the pointer to the start of particleMatrix?

Consider the case where realloc() is unable to expand the block of memory in place, and has to allocate a new block and copy over. Without another level of indirection, the caller's pointer will continue to point to the old block of memory.

[KBriggs]

I just fixed that. Now the code is:

double **readTimeStep(FILE *input, double **particleMatrix,int N)
{
  int i;

  particleMatrix = (double**) realloc(particleMatrix, N * sizeof(double*));
  if (particleMatrix == NULL)
  {
    printf("error allocating memory for particleMatrix");
    abort();
  }
  for (i = 0; i < N; i++)
  {
    particleMatrix[i] = (double *) realloc(particleMatrix[i], 4 * (sizeof(double)));
    if (particleMatrix[i] == NULL)
    {
      printf("error allocating memory for particleMatrix[i]");
      abort();
    }
  }  
  ///
  for (i = 0; i < N && fscanf(input,"%*s %lf %lf %lf %lf",&particleMatrix[i][0],&particleMatrix[i][1],&particleMatrix[i][2],&particleMatrix[i][3]) != EOF; i++)
  {
    ;
  }

 return particleMatrix; //reallocate matrix for each N during loop
}

It gives the same error still... Rawr.

[laserlight]

Did you #include <stdlib.h>?

[KBriggs]

yeah I did

[laserlight]

Post the smallest and simplest program that demonstrates the error.

[KBriggs]

This just segfaults, but most likely for the same reason...

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

double **readTimeStep(FILE *input, double **particleMatrix,int N)
{
  int i;

  particleMatrix = (double**) realloc(particleMatrix, N * sizeof(double*));
  if (particleMatrix == NULL)
  {
    printf("error allocating memory for particleMatrix");
    abort();
  }
  for (i = 0; i < N; i++)
  {
    particleMatrix[i] = (double *) realloc(particleMatrix[i], 4 * (sizeof(double)));
    if (particleMatrix[i] == NULL)
    {
      printf("error allocating memory for particleMatrix[i]");
      abort();
    }
  }  
  ///
  for (i = 0; i < N && fscanf(input,"%*s %lf %lf %lf %lf",&particleMatrix[i][0],&particleMatrix[i][1],&particleMatrix[i][2],&particleMatrix[i][3]) != EOF; i++)
  {
    printf("%f\t%f\t%f\t%f\n",particleMatrix[i][0],particleMatrix[i][1],particleMatrix[i][2],particleMatrix[i][3]);
  }

 return particleMatrix; //reallocate matrix for each N during loop
}

int main()
{
  double **particleMatrix;
  FILE *input;
  int N,i;
  double time;
  
  input = fopen("allbodys.xyz","r");
  if (input == NULL)
  {
    printf("input file open failed\n");
    abort();
  }
  fscanf(input,"%d",&N);
  fscanf(input,"%*s %*s %*d %*s %*s %*s %*s %*s %*s %*s %*s %lf",&time);
  particleMatrix = readTimeStep(input,particleMatrix,N);
  fclose(input);
  
  for (i = 0; i < N; i++)
  {
    free(particleMatrix[i]);
  }
  free(particleMatrix);
}

[laserlight]

You did not initialise particleMatrix in the main function. It should be initialised to be a null pointer.

[KBriggs]

Pretty sure NULL is the default no?

How do i make it NULL?

particleMatrix = NULL?

EDIT:

adding that line just makes the program segfault instead of giving me the big error backtrace. Which could be a good thing.

[matsp]

the original particleMatrix is not set to NULL, so the first time you call realloc() it thinks that there is already some memory allocated - you are now in a place called "undefined behaviour land" - which is a strange place somewhat resembling the "improbability drive" in the Hitchhikers guide to the Galaxy series - anything can (and will) happen. Unfortunately, unlike the improbability drive, nothing good comes out of this place. Just weirdness and badness. In this case, it's likely that realloc decides to free the memory that wasn't allocated and it probably gives you the error message posted in that attempt.

--
Mats

[dwks]

realloc() doesn't automatically initialize its memory to zero. So if N becomes bigger, you'll make particleMatrix bigger, but particleMatrix[something] .. particleMatrix[last] will be uninitialized. You'll then pass these uninitialized values to the second realloc(), and your program will crash.

There are two ways to fix this:

1. Simply initialize the newly realloc()'d memory to zero. (Not the best solution, but it works.)
2. Use malloc() the second time around, and instead of looping with "for (i = 0; i < N; i++)", only loop over the elements you have newly allocated as the array became larger.

I would second laserlight's opinion above and use something like this.

struct particleMatrixRow_t {
    double *col;
};

struct particleMatrix_t {
    struct particleMatrixRow_t *row;
    size_t rows, cols;
};

I'm assuming here that your matrix is square, so you can store one "cols" variable for the whole matrix. If this is not the case, simply store the cols in the particleMatrixRow_t structure.

This has several advantages:

• It's easy to pass around a structure.
• You know how much memory was previously allocated, so you know which parts of the structure are new, so you know which parts need malloc()ing.
• It's a lot more easily expandable.

[edit] Wow. There were about eight replies while I was typing up this one! I think this post is still applicable, though, so I'll leave it be. [/edit]

[laserlight]

Pretty sure NULL is the default no?

No, if you did not initialise, you did not initialise.

How do i make it NULL?

particleMatrix = NULL?

Yes, you should initialise it using a null pointer constant like NULL.

EDIT:
dwks' has a point: you would then have the same problem, but with the inner pointers.

[KBriggs]

With my compiler, the output for this code is "yes." Doesn't that mean that NULL is the default already?

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

int main()
{
  double **particleMatrix;
  
  if (particleMatrix == NULL)
  {
    printf("yes\n");
  }
  else 
  {
    printf("no\n");
  }
}