Search code examples
cmemory-managementmemory-leaksstructvalgrind

Structure and Linked List memory allocation valgrind error


I am currently working on a project where I am using valgrind to find memory leaks and I have some trouble trying to find them.

I've made a small little app that simulates where the problem is and I've managed to replicate the error I am seeing in valgrind.

Below is my main function:

int main(int argc, char** argv) {


    inboundStruct * inboundDataStruct = NULL;
    outboundStruct * outboundDataStruct = NULL;
    char *outboundName = NULL;
    if (mallocInboundStruct(&inboundDataStruct, 2))
    {
        printf("Error malloc'ing inbound struct\n");
        exit(1);
    }
    int i = 0;
    for (i = 0; i < 2; i++)
    {
        inboundDataStruct[i].index = i;
        asprintf(&inboundDataStruct[i].itemName, "Item %i", i);
        mallocOutboundStruct(&inboundDataStruct[i].outboundLeg);
        if (outboundDataStruct == NULL)
        {
            outboundDataStruct = inboundDataStruct[i].outboundLeg;
        }

        asprintf(&outboundName, "Outbound Target %i", i);
        insertOutboundLeg(&outboundDataStruct, outboundName, i);
        outboundDataStruct = NULL;
        free(outboundName);
        outboundName = NULL;
    }  
    printStructure(inboundDataStruct, 2);

    clearOutboundLinkedList(&outboundDataStruct);

    freeInboundStruct(&inboundDataStruct, 2);

    return (EXIT_SUCCESS);
}

Below is my malloc function for the inbound struct

int mallocInboundStruct(inboundStruct **inboundDataStruct, int size)
{
    int i = 0;
    inboundStruct *tempStruct = NULL;
    tempStruct = (inboundStruct*)malloc(size * sizeof(inboundStruct));
    if (tempStruct != NULL)
    {
        for (i = 0; i < size; i++)
        {
            tempStruct[i].index = i;
            tempStruct[i].itemName = NULL;
        }
        *inboundDataStruct = tempStruct;
        return 0;
    }
    return 1;
}

Below is my malloc for the outbound struct

int mallocOutboundStruct(outboundStruct **outboundDataStruct)
{
    outboundStruct *tempStruct = NULL;

    tempStruct = (outboundStruct*)malloc(sizeof(outboundStruct));
    if (tempStruct != NULL)
    {
        tempStruct->index = 0;
        tempStruct->nextLeg = NULL;
        tempStruct->outboundName = NULL;
        *outboundDataStruct = tempStruct;
        return 0;
    }
    return 1;
}

Below is free of the inbound struct

int freeInboundStruct(inboundStruct **inboundDataStruct, int size)
{
    inboundStruct *tempStruct = *inboundDataStruct;
    int i = 0;

    for (i = 0; i < size; i++)
    {
        free(tempStruct[i].itemName);
        tempStruct[i].itemName = NULL;
    }
    free(tempStruct);
    return 0;
}

Below is the freeing of the outbound struct

int clearOutboundLinkedList(outboundStruct **outboundDataStruct)
{
    outboundStruct *currentStruct = *outboundDataStruct;
    outboundStruct *temp;

    if (currentStruct == NULL)
    {
        return 0;
    }
    while (currentStruct->nextLeg != NULL)
    {
        temp = currentStruct;
        currentStruct = currentStruct->nextLeg;
        free(temp->outboundName);
        temp->outboundName = NULL;
        free(temp);
    }
    free(currentStruct->outboundName);
    currentStruct->outboundName = NULL;
    free(currentStruct);
    currentStruct = NULL;
    return 0;
}

and finally below is the function for the inserting data into the outbound struct

void insertOutboundLeg(outboundStruct ** outboundDataStruct, char * outboundName, int index)
{
    outboundStruct **ptr = outboundDataStruct;
    outboundStruct *currentLeg = *outboundDataStruct;

    if (currentLeg->outboundName == NULL)
    {
        currentLeg->index = index;
        //asprintf(&currentLeg->outboundName, "Item %s-%i", outboundName, index);
        currentLeg->outboundName = strdup(outboundName);
        *ptr = currentLeg;
    }
    else
    {
        while (currentLeg)
        {
            ptr = &currentLeg->nextLeg;
            currentLeg = currentLeg->nextLeg;
        }
        if (currentLeg)
        {
            if (currentLeg->outboundName != NULL)
            {
                free(currentLeg->outboundName);
                currentLeg->outboundName = NULL;
            }
            //asprintf(&currentLeg->outboundName, "Item %s-%i", outboundName, index);
            currentLeg->outboundName = strdup(outboundName);
            currentLeg->index = index;
            *ptr = currentLeg;
        }
        else
        {
            currentLeg = malloc(sizeof(*currentLeg));
            currentLeg->nextLeg = NULL;
            //asprintf(&currentLeg->outboundName, "Item: %s-%i", outboundName, index);
            currentLeg->outboundName = strdup(outboundName);
            currentLeg->index = index;
            *ptr = currentLeg;
        }
    }
}

When I run the app through valgrind it gives me the following output:

> ==32080== Command: ./mallocTest
> ==32080== Index: 0 Item: Item 0
>         Index: 0 Item: Outbound Target 0 Index: 0 Item: Item 1
>         Index: 1 Item: Outbound Target 1
> ==32080==
> ==32080== HEAP SUMMARY:
> ==32080==     in use at exit: 60 bytes in 4 blocks
> ==32080==   total heap usage: 13 allocs, 9 frees, 534 bytes allocated
> ==32080==
> ==32080== 36 bytes in 2 blocks are indirectly lost in loss record 1 of 2
> ==32080==    at 0x40072D5: malloc (vg_replace_malloc.c:291)
> ==32080==    by 0xB0798F: strdup (in /lib/libc-2.12.so)
> ==32080==    by 0x804869D: insertOutboundLeg (main.c:86)
> ==32080==    by 0x80485FF: main (main.c:38)
> ==32080==
> ==32080== 60 (24 direct, 36 indirect) bytes in 2 blocks are definitely lost in loss record 2 of 2
> ==32080==    at 0x40072D5: malloc (vg_replace_malloc.c:291)
> ==32080==    by 0x80488A4: mallocOutboundStruct (main.c:157)
> ==32080==    by 0x80485A3: main (main.c:31)
> ==32080==
> ==32080== LEAK SUMMARY:
> ==32080==    definitely lost: 24 bytes in 2 blocks
> ==32080==    indirectly lost: 36 bytes in 2 blocks
> ==32080==      possibly lost: 0 bytes in 0 blocks
> ==32080==    still reachable: 0 bytes in 0 blocks
> ==32080==         suppressed: 0 bytes in 0 blocks
> ==32080==
> ==32080== For counts of detected and suppressed errors, rerun with: -v
> ==32080== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 12 from 8)

I think the problem might be in my insertOutboundLeg but I can't see why. I'm new to C and am learning it as I go along.


Solution

  • Valgrind tells you where the lost memory was allocated; it cannot tell you where that memory ceases to be reachable. The problem is usually not with the allocation, but rather with how the allocated memory is handled afterward.

    In your case, the first loss record tells you that a chunk of memory allocated in insertOutboundLeg() via a call to strdup() was leaked. I have no line numbers to guide me, but it looks like that would have to be one of the several appearances of

    currentLeg->outboundName = strdup(outboundName);
    

    Remember always that strdup() allocates memory that the caller is responsible for freeing. I see that you do properly free this member in your function clearOutboundLinkedList(), so it must be that an outboundStruct somewhere goes out of scope or gets freed without being processed by that function.

    Looking carefully, then, I see that your main() function calls mallocOutboundStruct() twice, preserving exactly one of the resulting pointers in variable outboundDataStruct. That one is later freed via clearOutboundLinkedList(), but the other one never is. In fact, the other is never freed at all, which appears to be the subject of the second loss record.