Search code examples
cmalloclinaro

Dynamic array inside struct and malloc fail


Consider the following abstracted code that reads some bytes from a file:

typedef struct A{
int size;
char * dataArray;
}A

A load(char* filename, int inSize)
{
    A newA;
    newA.size = inSize;
    FILE *filePtr;
    filePtr = fopen(filename,"rb");

    char buff[1];
    int i = 0;

    newA.dataArray = ( char*)malloc(sizeof(char) * newA.size);
    for (i = 0; i < newA.size; i++)
    {
        fread(buff, sizeof(char), 1, filePtr);
        newA.dataArray[i] = buff[0];
    }

    char* copyOfDataArray = (char*)malloc(sizeof(char) * newA.size);

    for (i = 0; i < newA.size; i++)
    {
        fread(buff, sizeof(char), 1, filePtr);
        copyOfDataArray[i] = newA.dataArray[i];
    }

    newA.dataArray = copyOfDataArray;
    return newA
}

void Initialize()
{
    A first = load("file1", 100);
    A second = load("file2", 20);
}

Both calls to function load return the expected result (data array has the same bytes as the file). Variables first and second are never used again.

However after a couple of hundreds lines of code the program always crashes with:

*malloc.c:2451: sYSMALLOC: Assertion '(old_top == (..... failed.*

The crash always occurs on the same line of code, but that line has nothing to do with variables first, second or even with struct A whatsoever.

My question is: is my way of instancing and loading 'first' and 'second' wrong? Can it cause some kind of memory leak / memory overflow that crashes the program long after the load function has finished?

Bonus: The crash does not occur if I only load "file1", as soon as i load both "file1" and "file2" the crash reappears.

Sorry for the long question.


Solution

  • You have memory leaks there. You have to free the previously allocated memory in newA.dataArray, before you assign there a new memory.

    As stated by Joachim, read operation is very time consuming and you shall read data in blocks to minimize overhead.

    Additionally, you have to close file descriptors, otherwise they will be depleted soon.