Segfault while printing linked list after stack clear

Question

To be more specific, this code is supposed to be a lesser clone of the Unix function dc. The linked list seems to be working fine. If I attempt to use c to clear the memory, add more numbers, then print again with f I get a segfault. It seems to be printing what should be the Null Node in the linked list.

Interaction:
$ ./test
1 2 3
f
3
2
1
c
4 5
f
5
4
0
Segmentation Fault

Here's the code itself:

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

struct Node{
  int val;
  struct Node *next;
};

void cons_node(struct Node **head, int num)
{
  struct Node *newNode = malloc(sizeof(struct Node));
  newNode->val = num;
  newNode->next = NULL;
  if (*head == NULL){
    *head = newNode;
  }
  else {
    newNode->next = *head;
    *head = newNode;
  }
}

I'm assuming the problem lies here in the display func:

void display_list(struct Node *head)
{
  struct Node *current = head;
  while(current != NULL)
    {
      printf("%d\n", current->val);
      current = current->next;}

}

void print_top(struct Node *head)
{
  printf("%d\n", head->val);
}

or here in the clear func:

void clear_stack(struct Node *head)
{
  struct Node *current;
  while ((current = head)!= NULL) {
    head = head->next;
    free(current);
  }
}

void vorpal_func(struct Node *head)
{ 
  struct Node *current;
  current = head;
  free(current);
}
int main(){

  int input;
  int first = 1;
  char quit = 'n';
  char inputchar = ' ';

  struct Node *head = NULL;

  while (quit == 'n'){

    if (scanf("%d", &input) == 1){
      cons_node(&head, input);
      first = 0;
    }

    else{
      inputchar = getchar();

      if(first == 1)
    printf("List is empty\n");

      else{
    switch (inputchar){
    case 'q':
      quit = 'y';
      break;
    case 'E':
      quit = 'y';
      break;
    case 'f':
      display_list(head);
      break;
    case 'p':
      print_top(head);
      break;
    case 'c':
      clear_stack(head);
      first = 1;
      break;
    case 't':
      vorpal_func(head);
      break;
    }
      }
    }
  }

  return 0;
}

I've been attempting to figure out the problem for a few hours now. Any tips?

struct Node *newNode = (struct Node*)malloc(sizeof(struct Node)); - please please don't cast the result of malloc, — Tom Tanner, Sep 09 '15 at 10:24
@TomTanner, Hmm that bit was added due to my professor doing it that way. I can remove it, but can I ask why? — Blake Howard, Sep 09 '15 at 10:29
See here http://stackoverflow.com/questions/1565496/specifically-whats-dangerous-about-casting-the-result-of-malloc - it's unnecessary and can cause strange runtime errors. And it makes the code harder to read — Tom Tanner, Sep 09 '15 at 10:34

Tom Tanner · Accepted Answer · 2015-09-09T10:38:21.153

2

You don't clear your head after calling clear_stack, so when you add your next node, the next pointer is set to something that points into memory that has been freed. Or you could pass a pointer to head to clear_stack if you preferred.

void clear_stack(struct Node **head)
{
    while (*head != NULL) 
    {
        Node *current = *head;
        *head = current->next;
        free(current);
    }
}

In passing, cons_node could be written like this

void cons_node(struct Node **head, int num)
{
    struct Node *newNode = malloc(sizeof(struct Node));
    newNode->val = num;
    newNode->next = *head;
    *head = newNode;
}

edited Sep 09 '15 at 10:38

answered Sep 09 '15 at 10:31

Tom Tanner

9,244
3
33
61

Thanks for the clean up of cons_node, and the advice. And yep you were right. I need to sleep, haha. Thank you. – Blake Howard Sep 09 '15 at 10:40
If it's not too much trouble, could you throw out a guess as to why vorpal_func is seg faulting as well? – Blake Howard Sep 09 '15 at 11:01
@BlakeHoward it is good not to pass your head pointer directly to function. It those function modifies the head you won't get the starting of the linked list. Instead take head value in temp variable and pass those. – Nakul Sep 09 '15 at 11:36
@BlakeHoward not sure where / how it's being called, I can only presume it's being passed a bad pointer – Tom Tanner Sep 09 '15 at 14:50

Segfault while printing linked list after stack clear

1 Answers1