Why is my loop creating extra linked list nodes?

Question

I'm working with linkedlist. I want to create a loop that allows the user to add nodes to the list. My output always has two extra blank nodes. I believe it has something to do with the way I am using the input functions to both take input and cancel the loop but I can't pin point where the problem is.

I've tried a variety of variations including terminating as loop expression and with while(1) terminating inside the loop.

I hope it doesn't matter that I'm using Ubuntu on Windows 10 but who knows.

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

typedef struct node {
  int val;
  struct node * next;
} node_t;

node_t * init_node();
void print_list(node_t * head);

int main(){
  node_t * new = init_node();
  node_t * head = new;
  int c;

  printf("\n\tAt any time Press 'N' to quit.\n");
  do{
    if(c == 'n' || c =='N') {
      break;
    }
    //__fpurge(stdin);

    printf("\n\tEnter a value for the new node: ");
    scanf("\n%d", &new -> val);
    new -> next = init_node();
    new = new -> next;

  } while(c = getc(stdin) != 'N' && c != 'n');

  printf("\n\t");

  print_list(head);

  return 0;
}

node_t * init_node(){
  node_t * temp = (node_t *) malloc( sizeof(node_t *) );
  temp -> next = NULL;
  return temp;
}

void print_list(node_t * head){
  node_t * current = head;
  printf("\n\t");
  while(current != NULL){
    printf("%d->",current -> val);
    current = current -> next;
  }
  printf("null\n");
}

With inputs: 1, 2, 3 ...

The desired output is:
> 1->2->3->null

The current output is:
> 1->2->3->0->0->null

Thanks in advance!

Don't cast the result of malloc. Don't add spaces around `->`. — Tordek, Aug 24 '19 at 02:20
You shouldn't name your variables suffixed with `_t`. They are reserved for special use. Here give a read to https://stackoverflow.com/questions/231760/what-does-a-type-followed-by-t-underscore-t-represent — Mihir Luthra, Aug 24 '19 at 02:22
Also I don't know what reason to give for this, but avoid giving name `new` to a variable. `new` is a keyword in `c++`. Most programmers reading your code with get a little confused on seeing it first. — Mihir Luthra, Aug 24 '19 at 02:24
You may find [Singly Linked List of Integers (example)](https://pastebin.com/R2AewR3A) helpful. — David C. Rankin, Aug 24 '19 at 02:36
The `if(c == 'n' || c =='N')` is redundant, since `c` won't be `n` at the start of the loop (but you should initialize it to something), and it won't be by the next time you loop since the `while` clause will get rid of it. — Tordek, Aug 24 '19 at 02:41

Tordek · Accepted Answer · 2019-08-24T03:00:11.200

Every loop, you do this:

new->val = user_input
new->next = new_node()
new = new->next

So, every time, you add a new, uninitialized node at the end of the list. This happens to be 0 in your current system, but need not be.

Initially your list contains:

[?] -> null

The ? represents uninitialized data, which happens to be 0, and [] represents the node new points to.

When you input 1 on your first loop, you:

change the ? into 1
create a new next node with uninitialized data
make new point to it

So your list contains

1 -> [?] -> null

Then you input 2 and get:

1 -> 2 -> [?] -> null

Finally, print_list will perform like this:

[1] -> 2 -> ? -> null

Prints `1 ->`

1 -> [2] -> ? -> null

Prints `2 ->`

1 -> 2 -> [?] -> null

Prints `0 ->` // But could print anything.

1 -> 2 -> ? -> [null]

Breaks the loop and prints `null`

Additionally, your malloc is asking for space for node_t *, which is a pointer to your data structure; you should be calling malloc(sizeof(node_t)) or malloc(sizeof(*temp)). It is possible you're overwriting something by accident.

And I will assume the second zero comes from the way you're using your program: If you are pressing: 1, enter, enter,2, enter, enter, 3, enter, enter, [nothing], enter, n, then scanf will receive an empty string and evaluate is as 0.

You should check scanf's return value: it reports how many fields were successfully matched.

A better way to handle user input might be:

while (1) {
    char user_input[BUFSIZE];
    fgets(user_input, BUFSIZE, stdin);
    if (sscanf(user_input, "%d", node->val)) {
        ...
    } else if (user_input[0] == 'n' || user_input[0] == 'N') {
        break;
    }
}

Preferably it would be `node_t *temp = malloc (sizeof *temp);` using the derefernced pointer to set the typesize eliminates all chance of mistake. — David C. Rankin, Aug 24 '19 at 02:43

Why is my loop creating extra linked list nodes?

1 Answers1