-1

I am beginner to coding and I started learning C recently.

I wrote this code myself after learning concepts recently. I need few suggestions which can optimize my code.

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

typedef struct Node{
    int data;
    struct Node* next;
}
node;

//function to add node at the front
void push(node** head_ref, int new_data){
    //allocate node
    node* new_node = (node*)malloc(sizeof(node));

    //put in new data
    new_node->data = new_data;

    //make new node as head 
    new_node->next = (*head_ref);

    //move the head to new node
    (*head_ref) = new_node;
}

//function to add node after a certain node
void insertAfter(node* prev_node,int new_data){

    //check if previous node is null
    if(prev_node == NULL){
        printf("the given previous node cannot be NULL");
        return;
    }

    //allocate node
    node* new_node = (node*)malloc(sizeof(node));

    //put in new data
    new_node->data = new_data;

    //Make next of new node as next of prev_node
    new_node->next = prev_node->next;

    //move the next of prev_node as new_node
    prev_node->next = new_node;
}

//function to add a new node at end
void append(node** head_ref, int new_data){
    //allocate data
    node* new_node = (node*)malloc(sizeof(node));

    node* last = *head_ref;

    //put in the new data
    new_node->data = new_data;

    //new node is going to be last to make its next null
    new_node->next = NULL;

    //If the Linked List is empty, then make the new node as head
    if(*head_ref == NULL){
        *head_ref = new_node;
        return;
    }

    //else move to last node
    while(last != NULL){
        last = last->next;
    }

    last->next = new_node;
    return;
}

void printList(node* nodee)
{
    while (nodee != NULL)
    {
        printf(" %d ", nodee->data);
        nodee = nodee->next;
    }
}

int main()
{
    //Start with the empty list
    struct Node* head = NULL;
  
    // Insert 6.  So linked list becomes 6->NULL
    append(&head, 6);
  
    // Insert 7 at the beginning. So linked list becomes 7->6->NULL
    push(&head, 7);
  
    // Insert 1 at the beginning. So linked list becomes 1->7->6->NULL
    push(&head, 1);
  
    // Insert 4 at the end. So linked list becomes 1->7->6->4->NULL
    append(&head, 4);
  
    // Insert 8, after 7. So linked list becomes 1->7->8->6->4->NULL
    insertAfter(head->next, 8);
  
    printf("\n Created Linked list is: ");
    printList(head);
    return 0;
}
halfer
  • 19,824
  • 17
  • 99
  • 186
  • Questions seeking debugging help should generally provide a [mre] of the problem, as a single code snippet. This allows other people to easily test your program, by simply using copy&paste. You have provided several code snippets. It is unclear whether merging these snippets will yield a [mre]. Meanwhile, someone else has edited the question to merge these snippets. – Andreas Wenzel Jun 30 '22 at 17:59
  • Think about your `while` loop in `append`. What value is `last` guaranteed to have for the loop to exit? What is the next line `last->next = new_node;` then going to do? – Cheatah Jun 30 '22 at 18:02
  • Have you tried running your code line-by-line in a debugger while monitoring the values of all variables, in order to determine in which line your program stops behaving as intended? If you did not try this, then you may want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) You may also want to read this: [How to debug small programs?](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) – Andreas Wenzel Jun 30 '22 at 18:03
  • I get no compiler warnings but the code does actually crash, somewhere in `insertAfter()`. If I comment out that function call, it then crashes in `printList()`. – Weather Vane Jun 30 '22 at 18:06
  • You want: `while (last->next != NULL) last = last->next; last->next->next = new_node;` – Craig Estey Jun 30 '22 at 18:31

1 Answers1

5

Observe the following code in the append function:

while(last != NULL){
    last = last->next;
}

last->next = new_node;

The exit condition for the while loop is that last must be NULL. Therefore the next line last->next is guaranteed to dereference a NULL pointer thus causing a segmentation fault.

Cheatah
  • 1,825
  • 2
  • 13
  • 21