Thread: glibc detected malloc(): memory corruption

I am getting the error: "glibc detected malloc(): memory corruption: 0xb7fd3008" when function int ** load_table(FILE * file, int size) starts under UNIX. It works nicely under Windows. Do you find any mistakes here in load_table function?

Code:

#include "mathalizer.h"

int multiply(int x, int y)
{
    return x*y;
}

int add(int x, int y)
{
    return x+y;
}

int ** create_table(int size, int (* oper) (int,int))
{
    int x; 
    int y; 
    int **array;
    if(size<1) return NULL;
    array = (int **)malloc(size*sizeof(int*));
    if(array==NULL) return 1;

    for(x=0;x<size;x++)
    {
        array[x]=(int*)malloc(size*sizeof(int));
    }
    
    for(x=0;x<size;x++) 
    {
        for(y=0;y<size;y++)
        {
            array[x][y]=(* oper)(x,y);
        }      
    }
	
    return array;
}

int save_table(FILE * fp, int ** array, int size)
{
    int x;
    
    for(x=0;x<size;x++)
    {
        fwrite(array[x],sizeof(int),size,fp);     
    }
    fclose (fp);
   
    return 1;        
}

int ** load_table(FILE * file, int size)
{
    int x;
    int fread_return_code_2;
    int **array2;
    array2 = (int **)malloc(size*sizeof(int*));
    if(array2==NULL) return 1;

    for(x=0;x<size;x++)
    {
        array2[x]=(int*)malloc(size*sizeof(int));
    }
	
    for(x=0;x<size;x++)
    {
        fread_return_code_2=fread(array2[x],sizeof(int),size,file);
    }
    if(fread_return_code_2!=size) return 1;
    fclose(file);
	
    return array2;
}

Can you post the code that calls load_table() as there doesn't seem to be anything wrong with it so maybe the way it's called may have something to do with it.

Following the flow of your program, you are first calling save_table(), and if that succeeds then you are calling load_table(). Unfortunately, during the call to save_table() you are closing the file. How are you expecting to read from a file that you've already closed?

Code:

int check_saveload(int size)
{
   ...
   if ((save_table(tfile, table, size)) != 1)
   {
      fprintf(stderr, "test_saveload: Unable to save\n");
      return 0;
   }

   if ((table = load_table(tfile, size)) == NULL)
   {
      fprintf(stderr, "test_saveload: Got NULL in stead of table\n");
      return 0;
   }
   ...
}

int save_table(FILE * fp, int ** array, int size)
{
    int x;
    
    for(x=0;x<size;x++)
    {
        fwrite(array[x],sizeof(int),size,fp);     
    }
    fclose (fp);
   
    return 1;        
}

int ** load_table(FILE * file, int size)
{
    int x;
    int fread_return_code_2;
    int **array2;
    array2 = (int **)malloc(size*sizeof(int*));
    if(array2==NULL) return 1;

    for(x=0;x<size;x++)
    {
        array2[x]=(int*)malloc(size*sizeof(int));
    }
	
    for(x=0;x<size;x++)
    {
        fread_return_code_2=fread(array2[x],sizeof(int),size,file);
    }
    if(fread_return_code_2!=size) return 1;
    fclose(file);
	
    return array2;
}

I should also mention that you have a potential memory leak in your load_table() function:

Code:

int ** load_table(FILE * file, int size)
{
    int x;
    int fread_return_code_2;
    int **array2;
    array2 = (int **)malloc(size*sizeof(int*));
    if(array2==NULL) return 1;

    for(x=0;x<size;x++)
    {
        array2[x]=(int*)malloc(size*sizeof(int));
    }
	
    for(x=0;x<size;x++)
    {
        fread_return_code_2=fread(array2[x],sizeof(int),size,file);
        // You probably want to return as soon as you discover
        // that one of the reads failed (instead of continuing to loop)
        // However, you are forgetting to free the memory you allocated
        // for array2!
    }
    if(fread_return_code_2!=size) return 1;
    fclose(file);
	
    return array2;
}

The functions save_table() and load_table() have no business closing the FILE* that they've been passed. Their jobs are to save the table to the given file, and load the table from the given file - nothing more. When you call fprintf() and pass it a FILE*, would you expect it to close the file for you? Hell no.

I also noticed that you have a destroy_table() function that you didn't provide. The curious part is that you aren't passing it anything more than the table (int**). How does the destroy_table() function know how many rows there are in your table? It NEEDS to know that because you have to loop through and free each row before you free the array of row pointers. In other words:

Code:

void destroy_table(int **array, int size)
{
  int i;

  for (i = 0; i < size; i++)
    free(array[i]);

  free(array);
}

And finally, and perhaps most importantly, COMPILE YOUR PROGRAMS WITH WARNINGS ENABLED. You have all sorts of minor issues in your program that should be throwing warnings. Returning ints from a function that should be returning a pointer (without casting), allowing control flow to reach the end of a non-void function, etc... If you really are a total newbie, this may be the most important thing you learn for a while. It also might not be a bad idea to learn how to use gdb to debug your programs. Proper use of a debugger can save you HOURS of time.

(to enable warnings with gcc compile using the -Wall flag)

arpsmack, thank you so much! It works greatly now.
Why is it necessary to release memory two times:
free(array[i]);
free(array);
I thought that free(array[0]) is the same as free(array).