0

I've been working on speller for about two weeks now and I've finally got the programm to work, but valgrind cam up with a few memory errors. I've managed to solve most of them on my own, but the leak in my unload function still eludes me. I will obviously keep trying on my own, but I would love it if someone could take a look with fresh eyes and tell me what I'm missing.

The valgrind error is:

56 bytes in 1 blocks are definitely lost in loss record 1 of 1: (file: dictionary.c, line: 127)

and this is my unload function:

// Unloads dictionary from memory, returning true if successful else false
bool unload(void)                                      //Line 115
{
    //Declarations
    int freed = 0;
    node *cursor = malloc(sizeof(node));
    if (cursor == NULL)                                     //Line 120
    {
        printf("Memory Error: Unloading.");
        free(cursor);
        return false;
    }                                                       //Line 125

    node *tmp = malloc(sizeof(node));                         //Line 127!!!!!!!!!!!!!!!!!
    if (tmp == NULL)
    {
        printf("Memory Error: Unloading.");                  //Line 130
        free(tmp);
        return false;
    }

                                                      //Line 135
    //Function
    for (int i = N; i > 0; i--)
    {
        cursor->next = table[i];
        while(cursor->next != NULL)                                     //Line 140
        {
            tmp = cursor;
            cursor = cursor->next;
            free(tmp);
        }                                               //Line 145
        freed++;
    }
    free(cursor);
    if (freed == N)
    {
        return true;
    }
    else
    {
        return false;
    }
}
0
3

Complicated code in terms of memory management. This set of problems deals with the traditionally more difficult concepts of C programming, so we need a good theoretical basis for dealing with them. We must distinguish between a pointer that stores a memory address only, and the declaration of a new node in the linked list

"node * tmp = malloc (sizeof (node));"

this last statement reserves memory for a new node in the linked list, this is not necessary, a pointer to node is enough to access each member of the corresponding nodes (see linked lists), this type of pointer does not need us to reserve memory , you just have to declare it in the appropriate way. Knowing this, we must now "target" the appropriate item, usually at table hash.

node * cursor = hashtable [i];

Then we do the operations of freed the nodes, for this we use a temporary pointer, which points to the same place as the cursor, we assign the cursor to next, so as not to lose the linked list, and free temp, since temp points to an address of specific memory, free temp has the effect of freeing the memory assigned to that address. Another problem I see is how the hash table is traversed, look at your for loop:

for (int i = N; i> 0; i--)

Keep in mind that the array of pointers goes from zero to N-1, therefore when i = N we are outside the limits of the array, with everything said ALL your code could be summarized as:

for (int i = 0; i < N; i++)
{
    node* cursor = hashtable[i];
    while(cursor != NULL)                                   
    {
        node* tmp = cursor;
        cursor = cursor->next;
        free(tmp);
    }                                            
}
1
  • I would add, creating a temporary pointer, as @MARS says, doesn't require a malloc. However, if you don't immediately assign it to something like an element in hashtable[], then you should always immediately set it to NULL, node* tmp = NULL; This prevents that pointer from being accidentally used before something is assigned to it. Remember, the declaration and initialization are two different acts, even if done in the same line of code. – Cliff B May 28 '20 at 21:25
0
 // this whole block is not necessary, and you can delete it. 
 node *tmp = malloc(sizeof(node));                        
    if (tmp == NULL)  
    {
        printf("Memory Error: Unloading.");                
        free(tmp);
        return false;
    }

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .