Can you point out mistakes in this noob C code for an attempt to create a linked list

Question

it is my first attempt to create a linkedList. the code is not proper for sure but all i want to do is just to be a able to create a list and initialize it with one node for the start. the below code is syntactically correct but it is not working. can point the mistake.

    #include <stdio.h>
#define SIZE 5
struct node
{ int item;
  struct node *link;
};
struct linkedlist
{
    struct node *head;
    int count;

};
void init(struct linkedlist *p , int key)
{
    struct node *newnode;
    newnode = (struct node*)malloc(sizeof(struct node));
    newnode->link = NULL;
    newnode->item = key;
    p->head = newnode;
    p->count = 1;

}
void main()
{   struct linkedlist *s;
    init(s , 2);
    printf("%d", s->count);


}

You should declare an instance of the struct: `struct linkedlist s;` and then pass the address of `s` to the `init` function: `init(&s, 2);` The `printf` would then need to use dot notation: `printf("%d\n", s.count);` — user3386109, Jun 12 '21 at 05:49

score 2 · Accepted Answer · answered Jun 12 '21 at 05:45

You must allocate a structure and assign its pointer to s before having the function init dereference it.

Also you should use standard int main(void) in hosted environment instead of void main(), which is illegal in C89 and implementation-defined in C99 or later, unless you have some special reason to use non-standard signature.

Another note is that casting results of malloc() family is considered as a bad practice.

int main(void)
{   struct linkedlist *s = malloc(sizeof(*s)); /* allocate the structure */
    if (s == NULL) return 1; /* check if allocation succeeded */
    init(s , 2);
    printf("%d", s->count);

}

disclaimer: I didn't free s because it is allocated only once and the execution soon ends. The node is also not freed. Freeing at end of program won't be requird on modern OS. (c - What REALLY happens when you don't free after malloc? - Stack Overflow) You may want to add freeing for satisfying memory checkers like Valgrind.

score 0 · Answer 2 · edited Jun 12 '21 at 05:56

0

In the main function before calling the init function you need to allocate the memory to s pointer. So the solution will be to add following line before calling the init() function:

s = (struct linkedlist*)malloc(sizeof(struct linkedlist));

And it should work properly with no errors. Happy learning!

edited Jun 12 '21 at 05:56

MikeCAT

73,922
11
45
70

answered Jun 12 '21 at 05:54

No Man

113
6

score 0 · Answer 3 · answered Jun 12 '21 at 08:34

There is a major conceptual error in this program. If you want to pass an uninitialized pointer as as argument then initialize it to at least NULL. Passing an empty pointer doesn't make any sense. Just declaring a pointer to struct linkedlist in above code doesn't really get you an object of it unless you create one either statically or dynamically. Pointer is used to store a valid memory address and just declaring a pointer to int data type does not really create a variable of data type int for you. I hope the last three statements have made it clear what your mistake is. However, I have fixed your problem and the code is as follows:

#include <stdio.h>
#include <stdlib.h>
#define SIZE 5
struct node
{ 
   int item;
   struct node *link;
};
struct linkedlist
{
   struct node *head;
   int count;

};

void init(struct linkedlist *p , int key)
{
   struct node *newnode;
   newnode = (struct node*)malloc(sizeof(struct node));
   newnode->link = NULL;
   newnode->item = key;
   p->head = newnode;
   p->count = 1;

}
void main()
{  
    struct linkedlist *s = (struct linkedlist*)malloc(sizeof(struct 
    linkedlist));
    init(s , 2);
    printf("%d", s->count);


 }

Can you point out mistakes in this noob C code for an attempt to create a linked list

3 Answers3