Insertion into linked list

Question

I wrote a program that inserts nodes into a linked list in descending order.But whenever I test my code with numbers 12,14,13,19,7 in this order.Whenever I entered 7 I took 7 is already in the list.But as easily seen 7 is not in the list before I inserted.After give this error,if I choose print option by typing 2 my program entered in an infinite loop.I can not see my mistake and I am very confused.

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

struct node {
   int content;
   struct node* nextLink;
};

typedef struct node NODE;

void print (NODE*);
int insertNode (NODE** head, int x);

int main (void)
{
   int num, choice;
   NODE* head;
   head = NULL;

   do {
      printf("\nPlease press 1 to insert or press 2 to print or press 0 to exit\n");
      scanf("%d", &choice);
      switch (choice) {
         case 0:
            return 0;
            break;

         case 1:
            printf("Enter an integer to insert into the linkedlist:  ");
            printf("\n");
            scanf("%d", &num);
            insertNode(&head, num);
            break;

         case 2:
            print(head);
            break;

         default:
            printf("You entered an invalid number\n");
            return 0;
            break;
      }
   } while (choice == 1 || choice == 2);

   return 0;
}

int insertNode (NODE** head, int i)
{
   NODE* newNode;
   newNode          = (NODE*)malloc(sizeof(NODE));
   newNode->content = i;
   NODE* temporary = *head;
   newNode->nextLink = NULL;

   if ((*head == NULL) || ((*head)->content) < i) {
      *head             = newNode;
      (*head)->nextLink = temporary;
   }
   else {
      do {
         if (((temporary->content) > i) && ((temporary->nextLink->content) < i)) {
            newNode->nextLink   = temporary->nextLink;
            temporary->nextLink = newNode;
            return;
         }
         else if (temporary->content == i) {
            printf("To be inserted value is already in the list\n");
            return;
         }
         temporary = temporary->nextLink;
      } while (temporary->nextLink != NULL);

      if (temporary->content == i) {
         printf("To be inserted value is already in the list\n");
         return;
      }

      temporary->nextLink = newNode;
   }
   return 0;
}

void print (NODE* head)
{
   if (head == NULL) {
      printf("\nLinkedList is empty \n");
   }

   while (head != NULL) {
      printf("%d ", head->content);
      head = head->nextLink;
   }
}

i think you want to do NODE *temporary= head; instead of *head — Cristian Toader, Mar 25 '11 at 21:13
No I used there a double pointer so I should write like this. — virtue, Mar 25 '11 at 21:16
oh sorry didn't see that. Your program seems to run just fine for your example. I only changed the return; to return 0; because i was getting a pretty obvious compiling error. — Cristian Toader, Mar 25 '11 at 21:35
I used gcc on Ubuntu and I did not take any compiling error because of return statement. — virtue, Mar 25 '11 at 21:39

score 0 · Answer 1 · answered Mar 25 '11 at 21:23

0

I compiled it and ran it and it seemed to work fine except for one thing. insertNode is defined to return an int, yet 3 of the return statements are void returns. To get it to compile, I changed them to return 0;. If you were able to compile and run it as is, then it could be the stack was getting destroyed by the inconsistent returns.

answered Mar 25 '11 at 21:23

Mark Wilkins

40,729
5
57
110

Thank you everyone.Especially to MacGucky.The code now works but how can you correct the code by only remowing empty lines?Is much empty lines a problem for compiler?This question is to MacGucky,becuse he edited my code and it works. – virtue Mar 25 '11 at 21:34
@virtue: I looked at his edits and I did not see any change that would affect the behavior. I might have missed something, but it looked completely like formatting changes to make the code more readable. So I don't know what the issue was. Did you change the return statements as I suggested? I'm curious if that had any affect. – Mark Wilkins Mar 25 '11 at 21:56
@virtue: I just run it through my code-formatter (uncrustify) and removed some empty lines manually. I compiled and tested your code and my code (both with the change that all `return;` in insertNode where changed to `return 0;`) and they both work as expected. I found no problems. – MacGucky Mar 25 '11 at 22:03

score 0 · Answer 2 · answered Mar 24 '12 at 14:09

Your code won't work, if the first two values to be inserted are in descending order. It would give segmentation fault.

For the insertion of 2nd element you need to be careful

So after if condition

else if (temporary->content > i && temporary->nextLink==NULL)
      (*head)->nextLink = newNode;

score 0 · Answer 3 · answered Mar 24 '12 at 15:39

Your code is doing too much. If you code it differently, there are no special cases (such as insert at the top, insert at the tail of the list).

int insertNode (NODE **head, int val)
{
   NODE *newnode;

   for ( ; *head; head = &(*head)->nextLink) {
        if ( (*head)->content == val) {
            printf("To be inserted value (%d)is already in the list\n", val);
            return 0;
            }
        if ( (*head)->content > val) break;
        }
   newnode = malloc(sizeof *newnode); // Maybe check return here ;-)
   newnode->content = val;
   newnode->nextLink = *head;
   *head = newnode;
   return 1;
}

Insertion into linked list

3 Answers3