CODEWITHSUNDEEP
cpointersmemorymemory-leaksvalgrind
mmswe
mmswe

Reputation: 707

Cannot find the memory leak. Staring for 1 hour

Following C code has memory leak. I'm staring it for one hour but can't find it. I've been able to narrow it down to that function but still no luck for improvement.

Can you help me to find it?

Any help is appreciated. Thank you!

void insert ( LISTNODEPTR *sPtr, char value[SIZE] ) {
    LISTNODEPTR newPtr, previousPtr, currentPtr;
    int cmp;

    newPtr = malloc(sizeof(LISTNODE));

    if ( newPtr != NULL ) {

        newPtr->data = malloc(sizeof(SIZE));

        strcpy(newPtr->data, value);
        newPtr->nextPtr = NULL;

        previousPtr = NULL;
        currentPtr = *sPtr;

        /* Comparision to detect & remove duplicates nodes */
        while ( currentPtr != NULL ) {
            cmp = strcmp(value, currentPtr->data);
            if (cmp < 0) {
                /* you're at the point where you need to add the node */
                break;
            } else if (cmp == 0) {
                /* value is equal, no duplicate is allowed, leave */

                // since it is not added, destroy!
                free(newPtr->data);
                free(newPtr);

                return;
            }

            previousPtr = currentPtr;
            currentPtr = currentPtr->nextPtr;
        }

            if ( previousPtr == NULL ) {
                newPtr->nextPtr = *sPtr;
                *sPtr = newPtr;
            }
            else{
                previousPtr->nextPtr = newPtr;
                newPtr->nextPtr = currentPtr;
            }

    }


}

Edit:

More of the code:

#define SIZE 1001

struct listNode {
    char *data;
    struct listNode *nextPtr;
};

typedef struct listNode LISTNODE;
typedef LISTNODE *LISTNODEPTR;

/* Function prototype */
void insert ( LISTNODEPTR *, char[SIZE] );

Valgrind:

==19906== LEAK SUMMARY:
==19906==    definitely lost: 12 bytes in 3 blocks
==19906==    indirectly lost: 0 bytes in 0 blocks
==19906==      possibly lost: 0 bytes in 0 blocks
==19906==    still reachable: 0 bytes in 0 blocks
==19906==         suppressed: 0 bytes in 0 blocks

If I turn sizeof(SIZE) to SIZE, then memory leaks become +3000.

Edit 2:

I do deallocate them in main()

while ( startPtr != NULL ){
    LISTNODEPTR tmp = startPtr;
    startPtr = startPtr->nextPtr;
    free(tmp);
}

Upvotes: 0

Views: 124

Answers (3)

wildplasser
wildplasser

Reputation: 44250

Simplified:

  • changed the value[SIZE] argument to a char*, since that is what it is
  • without the confusing (and invisible) typedef
  • malloc() is only called once you know you really need the new node
  • removed excessive variables and conditions caused by not using the pointer to pointer construct
  • used strdup(), because that is simpler
struct listnode {
        struct listnode *nextPtr;
        char *data;
        };

void insert ( struct listnode **pp, char *value ) {
    struct listnode *newPtr;
    int cmp;

        /* find place to insert in LL */
    while ( *pp) {
        /* Comparision to detect & remove duplicates nodes */
            cmp = strcmp(value, (*pp)->data);
                /* Duplicate: nothing to do ... */
            if (cmp == 0) return;
                /* you're at the point where you need to add the node */
            if (cmp < 0) break;
            pp = &(*pp)->nextPtr;
            }

    /* when you get here, *pp points to the pointer you want to change.
    ** *pp could even be NULL (if we are at the end of the list)
    */

    newPtr = malloc(sizeof *newPtr);    
    if ( !newPtr ) { barf(); return; }

    newPtr->data = strdup (value);
        /* you could check newPtr->data here, and if null: free(newPtr) */

        /* "steal" the pointer */
    newPtr->nextPtr = *pp;
        /* and inject the new pointer into the LL */
    *pp = newPtr;
    return;
}

Upvotes: 2

Klas Lindb&#228;ck
Klas Lindb&#228;ck

Reputation: 33283

You free the nodes, but not the data. 

while ( startPtr != NULL ){
    LISTNODEPTR tmp = startPtr;
    startPtr = startPtr->nextPtr;
    free(tmp->data);     // Line added
    free(tmp);
}

Upvotes: 4

Iharob Al Asimi
Iharob Al Asimi

Reputation: 53026

There is a memory leak because the pointers you allocate locally in the function are never accessible from outside the function.

Also malloc(sizeof(SIZE)) is wrong, you should change it to malloc(SIZE), and if it's a fixed value, you should proably declare the data member as char data[SIZE] instead of malloc()ing space for it.

SHORT ANSWER

The memory leak is due to your program not free()ing the malloc()ed memory.

Upvotes: 3

Related Questions