Freeing memory for a complicated data structure in C

77 views Asked by At

(This is an attempt to condense another question I have posted earlier. This is a large program so please bear with me, I have identified and distilled it to the components I am showing here.)

I am working with a dictionary data structure, which is a table, that is an array of linked lists. (dictionary -> table -> array -> (array[i] is a node in a linked list))

Defined like this:

// Data type for nodes in linked list
typedef struct list_node {
    char word[MAX_WORD_LEN]; // Word, as a null-terminated string
    struct list_node *next;  // next in chain     
} list_node_t;

// Data type for a hash table. Use chaining for collisions, so array of list_nodes
typedef struct {
    list_node_t **array; // base array for hash table
    unsigned int length; // Length of base array
} table_t;

// Data type for a dictionary
typedef struct {
    table_t *table;   // Hash table that stores words
    unsigned size;    // Number of words stored in the dictionary
} dictionary_t;

In the end I am attempting to free all the memory I allocate by iterating through this structure, freeing all the nodes, then array, then the table and finally the dictionary. I believe my implementation is achieving this, however valgrind errors persist.

Here is my freeing implementation:

void table_free(table_t *table) {
        list_node_t *node;
        list_node_t *tempNode;
        for (int i = 0; i < table->length; i++) {
            if (table->array[i] != NULL) {
                if (table->array[i]->next == NULL) {//if only one node in array[i]'s linked list
                    free(table->array[i]);
                    continue;
                } else {
                    node = table->array[i];
                    while(node->next != NULL) {
                        tempNode = node->next;
                        free(node);
                        node = tempNode;
                    }
                }
            }
        }

        for (int i = 0; i < table->length; i++) {
            if (table->array[i] == NULL) { //free the spots I missed
                free(table->array[i]);
            }
        }
        free(table->array);
        free(table);
}

void dict_free(dictionary_t *dict) {
    table_free(dict->table);
    free(dict);
}

My big question is when working with memory, is it sufficent to only deallocate it at the end of the program if you allocate it in functions?

I ask because the valgrind errors are pointing me to other functions as well. My logic is that if I was freeing everything at the end correctly then memory leaks should not be an issue.

Here is the error:

==4046703== HEAP SUMMARY:
==4046703==     in use at exit: 60,928 bytes in 448 blocks
==4046703==   total heap usage: 2,031 allocs, 1,583 frees, 306,448 bytes allocated
==4046703== 
==4046703== 60,928 bytes in 448 blocks are definitely lost in loss record 1 of 1
==4046703==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4046703==    by 0x109C84: insert_word_into_table (dictionary.c:56)
==4046703==    by 0x109DD5: dict_insert (dictionary.c:95)
==4046703==    by 0x10A2B1: read_dict_from_text_file (dictionary.c:239)
==4046703==    by 0x1095E1: main (spell_check.c:105)

One approach I have taken to resolve this is free in the insert_word_into_table(); function which looks like this:

int insert_word_into_table(table_t *table, const char *word){
    int hsh_code = hash_code(word);
    int duplicateWordFlag = 0;
    //initialize the new node for the word
    list_node_t *newNode = malloc(sizeof(list_node_t));

    if (table->array[hsh_code] == NULL) {//if nothing is in the array
        table->array[hsh_code] = newNode;
        table->array[hsh_code]->next = NULL;
        strcpy(table->array[hsh_code]->word, word);
    } else {//if there is a node there
        list_node_t* i = table->array[hsh_code];
        while (i->next != NULL) {
            if (strcmp(word, i->word) == 0){
                duplicateWordFlag = -1;
            }
            i = i->next;
        }
        i->next = newNode;//new way
        strcpy(i->next->word, word);
        i->next->next = NULL;

    }
    //free(newNode); //my attempt to free it
    return duplicateWordFlag;
}

This approach did seem to put me in the right direction and produced the below message:

==4047436== HEAP SUMMARY:
==4047436==     in use at exit: 0 bytes in 0 blocks
==4047436==   total heap usage: 2,031 allocs, 3,604 frees, 306,448 bytes allocated
==4047436== 
==4047436== All heap blocks were freed -- no leaks are possible

However, when I free in this function (uncommenting the free(newNode) line) this now gives rise to this error here:

==4047436== Invalid free() / delete / delete[] / realloc()
==4047436==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4047436==    by 0x10A03A: table_free (dictionary.c:161)
==4047436==    by 0x10A0F9: dict_free (dictionary.c:187)
==4047436==    by 0x109AEB: main (spell_check.c:229)
==4047436==  Address 0x4add510 is 0 bytes inside a block of size 136 free'd
==4047436==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4047436==    by 0x109DB2: insert_word_into_table (dictionary.c:75)
==4047436==    by 0x109DE1: dict_insert (dictionary.c:95)
==4047436==    by 0x10A2BD: read_dict_from_text_file (dictionary.c:239)
==4047436==    by 0x1095E1: main (spell_check.c:105)
==4047436==  Block was alloc'd at
==4047436==    at 0x4848899: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==4047436==    by 0x109C84: insert_word_into_table (dictionary.c:56)
==4047436==    by 0x109DE1: dict_insert (dictionary.c:95)
==4047436==    by 0x10A2BD: read_dict_from_text_file (dictionary.c:239)
==4047436==    by 0x1095E1: main (spell_check.c:105)

The above valgrind message gives me the sense that I am freeing the memory and then trying to read/write to it, but I am unsure how to address this.

Could someone please help shine a light on this issue? I feel like I am misunderstanding memory allocation in C. Again, is it not sufficient to just free everything at the end?

0

There are 0 answers