I'm getting back into C after a long break, and as always the problems lie in use of pointers and general memory management.

My specific problem arises from the use of realloc on a struct in order to dynamically grow it as an array as required. I've never really worked with struct arrays before so I do apologise in advance.

Note: I haven't gotten as far as adding any data to the struct data fields as of yet. I have also renamed all variables to generic names for the purpose of this example.

Here's my struct definition:

typedef struct a_struct
{
    char a;
    char b;
    int num;
} TheStruct;

I allocate the first struct via calloc:

TheStruct *new_structInstance(void)
{
    TheStruct *structInstance = NULL;

    structInstance = calloc(1, sizeof(TheStruct));

    return structInstance;
}

This is my attempt at reallocating the first struct into an array of them, which is where I'm almost certainly going wrong:

void add_structInstance_to_array(TheStruct *structInstance, int *numElements, 
int *bufferLen)
{
    if (*numElements == *bufferLen) {
        *bufferLen  = GROWBY; // #define 16
        TheStruct *newstructInstance = realloc(structInstance, 
*bufferLen * sizeof(structInstance));
        if (newstructInstance == NULL) {
            free(structInstance);
            printf("Error: Memory could not be allocated.");
            exit(EXIT_FAILURE);
        }
        else if(structInstance != newstructInstance) {
            structInstance = newstructInstance;
        }
        newstructInstance = NULL;
    }
    *numElements  = 1;
}

I free the array of structs thus:

void free_struct_array(TheStruct *structInstance, int *numElements)
{
    for(int i = *numElements-1; i >= 0; i--) {
        free(&structInstance[i]);
    }
    free(structInstance);
}

Note that the input text file input format read by fgets in main approximately:

char char int

And I drive the whole thing in main:

void main()
{
    TheStruct *structInstance = new_structInstance();
    int initialCreated = 0;

    int bufferLen = 0, numElements = 0;

    char buffer[BUFFER]; // #define 20

    FILE *file = fopen("input.txt", "r");

    while(fgets(buffer, sizeof(buffer), file) != NULL) {
        if(!initialCreated){
            initialCreated = 1;
        }
        else {
            add_structInstance_to_array(structInstance, &numElements, &bufferLen);
        }
    }

    // Free memory
    fclose(file);
    free_struct_array(structInstance, &numElements);
}

Compiling the code generates the following terminal output:

free(): double free detected in tcache 2 Aborted (core dumped)

Valgrind outputs the following (I've skipped more verbose output as it includes my filesystem folder structure):

==102002== LEAK SUMMARY:
==102002==    definitely lost: 128 bytes in 1 blocks
==102002==    indirectly lost: 0 bytes in 0 blocks
==102002==      possibly lost: 0 bytes in 0 blocks
==102002==    still reachable: 472 bytes in 1 blocks
==102002==         suppressed: 0 bytes in 0 blocks
==102002==
==102002== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

If there is any way I can output more useful information without exposing my file structure, please let me know.

I tried an "easier" version of this example, instead having a single struct and running realloc on an array within the struct, e.g.:

typedef struct a_struct
{
   int *arr;
}

The above worked fine. Modifying the example to use structs as the array elements instead, a bit like a linked list I suppose, is my intention, so that I can access the struct array such as:

a_struct[element].datatype = something;
int getInt = a_struct[element].inttype;

So I suppose the question is, is it possible using my approach above, and if so, where am I going wrong and is there a better approach?

CodePudding user response：

You not only free the individual elements, you also free the first element again.

This code is highly problematic:

void free_struct_array(TheStruct *structInstance, int *numElements)
{
    for(int i = *numElements-1; i >= 0; i--) {
        free(&structInstance[i]);
    }
    free(structInstance);
}

This should be a singular array of structs, it should be one allocation you can just free() in one shot. If it was an array of pointers it'd be TheStruct**.

Remember how you allocated:

structInstance = calloc(1, sizeof(TheStruct));

Your free() calls should pair up exactly with those.

Reminder: Never, ever, call free() on something you did not explicitly allocate¹

¹ Or that was allocated on your behalf by a malloc() family allocator, as some libraries do.

CodePudding user response：

Aha! Thanks to @tadman, I dug deeper into freeing allocated struct pointers inadvertently through going out of scope / not returning the pointer to the struct.

I fixed the problem by modifying my add_structInstance_to_array to return the pointer to the reallocated struct:

TheStruct *add_structInstance_to_array(TheStruct *structInstance, int *numElements, int *bufferLen)
{
    if (*numElements == *bufferLen) {
        *bufferLen  = GROWBY; // #define 16
        TheStruct *newstructInstance = realloc(structInstance, 
*bufferLen * sizeof(structInstance));
        if (newstructInstance == NULL) {
            free(structInstance);
            printf("Error: Memory could not be allocated.");
            exit(EXIT_FAILURE);
        }
        else if(structInstance != newstructInstance) {
            structInstance = newstructInstance;
        }
        newstructInstance = NULL;
    }
    *numElements  = 1;
    
    return structInstance;
}

To further agree with @tadman, I could also get rid of the free_struct_array function that would loop through individual struct elements, and instead simply free the struct array once in main.