Linked List pointer printing memory leak

Question

The error is that it prints out the memory instead the values of each node. I tried all combinations of pointers and different printing styles but they all show up as memory leaks.

Here is my code:

#include <stdio.h>//headers
#include <stdlib.h>


struct node{
    int val;
    struct node *next;
};//linked list

struct node *curr = NULL;//list pointers
struct node *prev = NULL;
struct node *head = NULL;

int main(){

    int i;

    struct node *curr = (struct node*) malloc(sizeof(struct node));
    head=curr;//sets head node

    for (i=1;i<=5;i++){

        curr->val=i;//sets data
        struct node *prev = (struct node*) malloc(sizeof(struct node));
        curr->next=prev;
        curr=prev;//links to previous node

        printf("%d\n", *curr);//prints out data
    }
    return 0;
}

Don't cast the result of `malloc` for some reasons described here: http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc (a bit offtopic advice) — yulian, Dec 30 '14 at 19:19
this line, in main, masks the global declaration of the same name/type and should not case the returned value from the malloc family of functions: 'struct node *curr = (struct node*) malloc(sizeof(struct node));' suggest using: 'curr = malloc(sizeof(struct node));' Similar considerations for the memory leak: 'struct node *prev = (struct node*) malloc(sizeof(struct node));' — user3629249, Dec 30 '14 at 19:48

score 3 · Answer 1 · answered Dec 30 '14 at 19:17

Well, you have no calls to free in your program, so yes, every call to malloc is technically a leak. That said, I think you are confusing your terms here.

If you want to print out the value of the node, use cur->val, not *cur. *cur returns a struct node, which you are telling printf to interpret as an int.

score 1 · Answer 2 · answered Dec 30 '14 at 19:18

1

You should print the value accessing it directly like this

printf("%d\n", curr->val);

answered Dec 30 '14 at 19:18

Iharob Al Asimi

52,653
6
59
97

score 1 · Answer 3 · answered Dec 30 '14 at 19:41

1

Plus you are not setting the value of prev at any point so curr = prev; and then printf("%d\n", curr->val); will just print rubbish.

answered Dec 30 '14 at 19:41

computador7

170
6

score 0 · Accepted Answer · answered Dec 30 '14 at 19:46

There are several things wrong apart from the way you access the val field, which as has been commented should be curr->val.

Your program, when thus corrected, prints rubbish because you only print it after pointing curr to prev, and val is uninitialised.

curr->val = i;
...
curr = prev;
printf ("%d\n", curr->val);

Also, you have declared curr and prev both globally and within the function.

I have simplified this, printing the list after it has been created. If you want a different sequence, reverse the i loop. Note I have removed prev.

#include <stdio.h>
#include <stdlib.h>

struct node{
    int val;
    struct node *next;
};

int main(){
    int i;
    struct node *curr, *head = NULL;

    // build the list
    for (i=1; i<=5; i++) {
        curr = malloc(sizeof(struct node));
        curr->val = i;
        curr->next = head;
        head = curr;
    }

    // print the list
    curr = head;
    while (curr) {
        printf ("%d\n", curr->val);
        curr = curr->next;
    }
    return 0;

}

You should check the return value from malloc() before using it.

score 0 · Answer 5 · answered Dec 30 '14 at 20:23

the following code compiles cleanly 
caveat: I have not run this code

#include <stdio.h>//headers
#include <stdlib.h>


struct node{
    int val;
    struct node *next;
};//linked list

struct node *curr = NULL;//list pointers
struct node *prev = NULL;
struct node *head = NULL;

int main()
{

    int i;


    for (i=1;i<6;i++)
    {
        // get block
        if( NULL == (curr = malloc(sizeof(struct node)) ) )
        { // then, malloc failed
            perror( "malloc failed" );
            exit( EXIT_FAILURE );
        }

        // implied else, malloc successful

        // fill in fields
        curr->val = i;
        curr->next = NULL;

        // display val
        printf("%d\n", curr->val);//prints out data

        if( NULL == head)
        { // then, first node
            head = curr;
            prev = curr;
        }

        else
        { // else, after first node
            // link in new node
            prev->next = curr; // link in new node
        } // end if
    } // end while

    prev = head;
    curr = prev;
    while( curr )
    { // then, node to free
        curr = prev->next;
        free(prev);
        prev = NULL;
    } // end while

    return 0;
} // end function: main

Linked List pointer printing memory leak

5 Answers5