wrong output in singly linked list in c

Question

I need a help. I tried to create a singly linked list. Everything is fine so far,but I got a wrong output. I have no idea about my mistake. Thank you!

My output is :

 create a single linked list
 list:
 aaaaa
 -1342177280

My code is :

struct node{

    int key;
    int val; 
    struct node *next;  

};

struct node *create_single_link(){

    struct node *head = malloc(sizeof(struct node));

    head->next = NULL;

    return(head);

}


void insertVal( struct node *lst, int v){

    struct node *current = malloc(sizeof(struct node));
    current -> val = v;

    current->next = lst;
    lst = current;

}

void print_list(struct node * head){


    struct node *ptr = head;

    printf("list:\n");

    while(ptr != NULL ){
        printf("aaaaa\n");
        printf("%d \n ",ptr ->val );
        ptr = ptr->next;
    }


}

int  main(int argc, char const *argv[])
{

    struct node *list;
    list = create_single_link();
    if(list != NULL){
        printf("create a single linked list\n");
    }
    else
        printf("failed to create a single linked list\n");

    insertVal(list,2222);
    //insertVal(list,2);
    print_list(list);


    return 0;
}

Possible duplicate of [How can I malloc a struct array inside a function? Code works otherwise](http://stackoverflow.com/questions/36075244/how-can-i-malloc-a-struct-array-inside-a-function-code-works-otherwise) — MikeCAT, May 12 '16 at 05:49
Using value in buffer allocated via `malloc()` and not initialized invokes *undefined behavior*. — MikeCAT, May 12 '16 at 05:51
You can read https://mortoray.com/2012/01/08/whats-an-object-whats-a-variable/, you should know the difference between `pass value` and `pass reference`. — BlackMamba, May 12 '16 at 07:30

score 3 · Answer 1 · answered May 12 '16 at 05:57

Problem 1

You have:

void insertVal( struct node *lst, int v){

    struct node *current = malloc(sizeof(struct node));
    current -> val = v;

    current->next = lst;
    lst = current;
}

That does not change the value of list in main. It only change where lst points to locally in the function.

I suggest changing the function to:

struct node* insertVal( struct node *lst, int v){

    struct node *current = malloc(sizeof(struct node));
    current -> val = v;

    current->next = lst;
    return current;
}

and using it as:

list = insertVal(list, 2222);

Problem 2

The value of the node created in create_single_link is left uninitialized. You are better off not using it at all. You can change main to:

int  main(int argc, char const *argv[])
{
   struct node *list = NULL;
   list = insertVal(list,2222);
   list = insertVal(list,2);
   print_list(list);

   return 0;
}

score 0 · Answer 2 · answered May 12 '16 at 05:59

0

insertVal modifies lst but does not return it.

This means only one item can be added.

You are printing out the head with u initialised v

answered May 12 '16 at 05:59

mksteve

12,614
3
28
50

Artur Opalinski · Answer 3 · 2016-05-12T06:30:40.940

A list should have its head, which points to the next element of the list. You have done the opposite: the rest of your list (the first data piece) points to the head (in insertVal())
The head usually already stores the first piece of data; you only use head as a pointer to the rest of the list, not for storing data.
The reason for the erroneous behavior observed by you is that in print_list() you assume the opposite to my items 1. and 2. above: you try to print out the val associated with the head (which is against item 2 above), and you assume that the remaining list elements follow head (which is against item 1 above). I would suggest leaving print_list() as is, but correcting the former two items.
BTW you may consider changing the wording of your question, as your problem is not with „wrong output” but with a list implementation in C. See http://www.cprogramming.com/tutorial/c/lesson15.html for an example implementation.
I am skipping for now that your struct node has two fields (key and val) yet your insertVal() only fills vall. While not an error per se, it is dangerous as sooner or later you will forget to assign a meaningful value to key.
Happy coding :-)

SWIIWII · Answer 4 · 2016-05-12T08:23:01.190

In fact, your fucntion insert value doesn't make any sense, if you mean to insert a value in the end of the linked list, I hope the code below can help you.

#include <stdio.h>
#include <stdlib.h>
struct node{
    int key;
    int val;
struct node *next;
};

struct node *create_single_link() {
struct node *head = (struct node*)malloc(sizeof(struct node));
head->next = NULL;
return (head);
}

void insertVal (struct node *lst, int v) {
while(lst->next != NULL) {
    lst = lst->next;
}
lst->next = (struct node*)malloc(sizeof(struct node));
lst->next->val = v;
lst->next->next = NULL;
}

void print_list(struct node *ptr) {
while(ptr != NULL) {
    printf("the key is %d and the value is %d\n", ptr->key, ptr->val);
    ptr = ptr->next;
}
}


void main(void) {

struct node *list;
list = create_single_link();
if(list != NULL) 
    printf("a single list has been created\n");
else
    printf("faied to create a single list\n");
list->val = 1111;
insertVal(list, 2222);
insertVal(list, 3333);
print_list(list);
}

wrong output in singly linked list in c

4 Answers4