0

I wanted to make a list using double pointer and using void as return.

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

typedef struct list{
   int value;
   struct list *next;
}*list;

void addnode(struct list **List, int number) {
   if(*List == NULL) {
      *List = (struct list*)malloc(sizeof(struct list*));
      (*List)->value = number;
      (*List)->next = NULL;
   } else {
      while((*List)->next != NULL) {
         (*List) = (*List)->next;
      } 
      *List = (struct list*)malloc(sizeof(struct list*));
      (*List)->value = number;
      (*List)->next = NULL;
   }
}

int main() {
   list List1 = NULL;

   addnode(&List1, 20);

   printf("%d \n", List1->value);

   addnode(&List1, 30);
   printf("%d \n", List1->value);
   printf("%d \n", List1->next->value);
   return 0;
}

The first if in addnode is always executed but i want to append the list if its not empty but it seems like it never work. Ill also get segmenation fault because in the last printf it tries to take the next element in the list but its never initialized like i want.

If everthing worked as i wanted i should have printed out

 printf("%d\n", List1->value) 

20

 printf("%d\n", List1->value)

20

printf("%d\n", List1->next->value)

30

Brian L
  • 3,201
  • 1
  • 15
  • 15
user2851726
  • 53
  • 1
  • 1
  • 4
  • can you [edit] your post to fix the indentation in `addNode`? it is currently a bit hard to read. – yhw42 Oct 06 '13 at 13:15

3 Answers3

0

The size you are passing to malloc is wrong.

You are allocating a struct list, not a struct list *.

6502
  • 112,025
  • 15
  • 165
  • 265
0

If you are trying to append a new list item, remember (*List)->next will already be NULL on the second call. The malloc following that uses the pointer before the NULL list item (*List) when it should be assigned to the next list item, the one that is NULL, to make it non-NULL ((*List)->next=malloc(struct list);).

Also, your malloc should be using sizeof(struct list), without the *. If you add the *, you're allocating a struct list **. A rule you can use is use one * fewer than the destination type as the sizeof operand. Since your destination is *List, which is of type struct list *, use sizeof(struct list). Alternatively, because your destination is *List, use sizeof **List (use one more * than the destination variable has). This avoids you needing to know the type. It won't matter if List or *List is NULL because the sizeof operation is executed first; pointer dereferencing never occurs since sizeof works on the type of the variable.

0

Modify your program like this

int addNode(struct list **List, int number)
{
    struct list *new, *tmp;             // new = create new node, tmp = navigate to last

    new = malloc(sizeof(struct list));
    if(!new) {                         //always validate "malloc"
            perror("malloc");
            exit(1);
    }

    new -> value = value;             // assigning values to new node
    new -> next = NULL;

    if(!(*list)) {               //Check if list is empty or not, plz initialize *list@main() with NULL as like your program. or write seperate function to initialize
            *list = new;  
            return 0;            //no need write else condition, bcoz its the first node. and u can directly return
    }

    tmp = *list;
    while(tmp -> next)            // To navigate to last node
            tmp = tmp -> next;
    tmp -> next = new;            //creating link to new node
    return 0;
}

It's better to write print function seperatly.

 int print(struct list **list)
 {
    struct *current;      //current is your current node position
    current  = *list;

    while(current) {       //loop till current node addr == NULL
            printf("%d\t", current -> value);
            current = current -> next;
    }

    printf("\n");
    return 0;
}
gangadhars
  • 2,584
  • 7
  • 41
  • 68