0

Can't find what is wrong with this code, it works as expected when inputting exactly 4 values, but on the fifth call (before it even asks for scanf) it always gives me this error: * glibc detected ./a2: double free or corruption (fasttop): 0x0916e018 **

Here's some code of my program:

typedef struct {
  int i;
  char str[25];
} typeX;

int main(){
  int dSize = 0;
  int *dSizePtr = &dSize;
  dPointer = (typeX **)malloc(sizeof(typeX *)); // makes an array of pointers
  int i;
  for (i = 0; i < 100; i++)
    makeElement(dPointer, dSizePtr); // Puts values into those pointers
  free(dPointer);
  return 0;
}

void makeElement(dPointer **, int *dSizePtr){
  dPointer = (typeX **)realloc(dPointer, sizeof(typeX *)*(*dSizePtr+1)); // grow the array by one
  if (typeX == NULL)
    return; // some kind of quit statement, just return for now

  dPointer[*dSizePtr] = (typeX *)malloc(sizeof(typeX)); // make a new pointer in the array
  scanf("%s", dPointer[*dSizePtr]->str); // input the values of the struct (have to use scanf)
  char input[20]; 
  scanf("%s", input);
  dPointer[*dSizePtr]->int = atoi(input);

  ++(*dSizePtr);
}

I know I don't have to make a dSizePtr and I can just pass in &dSize, but the way my program is currently set up (this isn't exactly the same, just compressed for readability), that's the way I have to pass it.

I honestly have no idea why this error is coming up. Been looking at my code for hours and reading online and haven't found a solution. Any help will be greatly appreciated!

Andrew B
  • 71
  • 1
  • 6
  • Quick tip: install Valgrind, then run your program with valgrind. It will give you lots of information about these kinds of errors, saving you hours of time looking for them. – Dietrich Epp Oct 21 '12 at 19:51

2 Answers2

1

The problem is that your function makeElement get the value of dPointer, not its reference. When you realloc the data, the originally allocated chunk is freed. But the dPointer outside of the makeElement scope is not changed;

The runtime error is delayed as the actual memory allocation is performed in quantities bigger than sizeof(typeX*)

Serge
  • 6,088
  • 17
  • 27
  • 1
    Comments: 1. don't cast the return value of `malloc()` 2. instead of `sizeof(type)`, `sizeof(variable)` is preferred, since the former will break if the type of the variable changes. –  Oct 21 '12 at 19:50
  • Well it initially is supposed to have only one pointer in it (an array of size 1), as it grows with every call to makeElement(). Shouldn't it work the same? – Andrew B Oct 21 '12 at 19:51
  • @H2CO3 I know that you are always arguing against this form. I don't want to escalate the debates :) – Serge Oct 21 '12 at 19:52
  • @Serge Neither do I, it's just better. –  Oct 21 '12 at 19:53
  • @AndrewB Please see the upate – Serge Oct 21 '12 at 20:00
  • Thanks for the update! Does this mean I will have to use a triple pointer? – Andrew B Oct 21 '12 at 20:20
  • In this case - yes. and better use some prefixes for local variables names and function argument names - this will help to avoid the shadowing of global declarations - the also possible source of bugs – Serge Oct 21 '12 at 20:23
0

This line is causing the double free.

dPointer = (typeX **)realloc(dPointer, sizeof(typeX *)*(*dSizePtr+1)); // grow the array by one

For the first few iterations of the loop in the caller the block of memory is large enough that realloc() doesn't have to do anything, and thus it returns the same pointer passed to it. But at some point the block of memory is too small and realloc() has to allocate a new block of memory and returns a pointer to it. That returned pointer is assigned to dPointer in makeElement() but it does not change the value of dPointer in the caller. So the caller continues to pass the old dPointer value into makeElement(), which passes it to realloc(), which notices that this pointer has been freed (by the call to realloc() that expanded the size of the array).

Kyle Jones
  • 5,492
  • 1
  • 21
  • 30