#Implementing a linked list

1 messages · Page 1 of 1 (latest)

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

typedef struct NODE {
    int data;
    struct NODE* next_node;
}NODE;

NODE* create_list(void) {
    NODE* new_node=malloc(sizeof(NODE));
    new_node->next_node=NULL;
    new_node->data=0;
    return new_node;
}

int insert_at_beginning(NODE* first_node, int data) {
    NODE* new_node=malloc(sizeof(NODE));
    if (new_node==NULL) {
        printf("error allocating memory");
        return -1;
    }
    if (first_node==NULL) {
        printf("invalid linked list passed");
        return -2;
    }
    new_node->data=data;
    new_node->next_node=first_node; //The next_node points to the preivous first node.
    first_node=new_node; //New node is set to the first node.
    return 0;
}

int print_list(NODE* first_node) {
    NODE* buffer=first_node;
    while (buffer!=NULL) {
        printf("%i\n", buffer->data);
        buffer=buffer->next_node;
    }
    return 0;
}


int main(void) {
    NODE* new_table=create_list();
    int num=5;
    int num1=10;
    int num2=15;
    insert_at_beginning(new_table, num);
    insert_at_beginning(new_table, num1);
    insert_at_beginning(new_table, num2);
    print_list(new_table);
}

trying to make a simple implementation of a linked list, but the print_list function just prints out 0. Where might be the problem ??

ebon sageBOT
#

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

cerulean moss
distant bear
#

other random remarks:

  • you test for malloc null return in insert_at_beginning, but not in create_list. that's not really coherent (some say "malloc cann't dramatically fail because of automatic segfault on some architectures", but that's UB anyway to deref a null pointer, and the segfault is not portable. the proper way to handle UB is to not rely on how it is supposed to look like

  • in insert_at_beginning, if the input is null, you still allocated for the new_node. that's a memory leak

  • usage of sizeof(NODE) is a poor coding style that doesn't embrace the single source of truth principle. it's preferable to do

NODE* new_node=malloc(sizeof(*new_node));

which is valid C (even though it might feel counter-intuitive because new_node is not yet declared. it makes sense because it is actually

NODE* new_node;
new_node = malloc(sizeof(*new_node));
  • there is code repetition between insert_at_beginning and create_list. assuming insert_at_beginning is currently buggy and incomplete, I won't suggest right now how to fix that, but it would be reasonnable to have a single function that actually malloc and initialize, based on inputs
foggy gazelle
distant bear
#

think about hisarya's comment

foggy gazelle
#

i still dont get it.... 😓

distant bear
#

insert_at_beginning should return new_node

foggy gazelle
#

ohh alright got it

#

but also

int insert_at_beginning(NODE** first_node_ptr, int data) {
    NODE* new_node=malloc(sizeof(NODE));
    if (new_node==NULL) {
        printf("error allocating memory");
        return -1;
    }
    if (*first_node_ptr==NULL) {
        printf("invalid linked list passed");
        return -2;
    }
    new_node->data=data;
    new_node->next_node=*first_node_ptr; //The next_node points to the preivous first node.
    *first_node_ptr=new_node; //New node is set to the first node.
    return 0;
}

i also fixed it like this. Is a better or a worse solution, and why ?

#

i passed a pointer to the node pointer

distant bear
#

looks fine

#

(you still suffer from all the issues I've listed above)

foggy gazelle
#

ohh hmm ill take care of thatt thank youu

foggy gazelle
#

!solved

ebon sageBOT
#

Thank you and let us know if you have any more questions!

This thread is now set to auto-hide after an hour of inactivity

brazen agate