Search code examples
clinked-listpass-by-referencesingly-linked-listfunction-definition

Fixing load of null pointer of type 'Node *' error


I am working on implementing a Linked List data structure in C. Below are my current functions for my Linked List implementation file (llist.c)

#include "llist.h"

// Frees all allocated memory associated with the list pointers iteratively
void deleteList(Node **list) {
    Node* ptr = *list;
    Node* temp;

    while(ptr != NULL) {
        free(ptr->data);
        temp = ptr;
        ptr=ptr->next;
        free(temp);
    }
}

// Frees all allocated memory associated with a single node

void deleteNode(Node **toDelete) {
    Node * del = *toDelete;
    free(del->data);
    free(del);
}

// Allocates memory for a new string and returns a pointer to the memory

Node *newNode(char *string) {
    unsigned long len = strlen(string);
    printf("length : %lu \n\n", len);

    Node *temp = (Node*)malloc(sizeof(Node));
    temp->data = (char*)malloc(len + 1*sizeof(char));
    strcpy(temp->data, string);
    temp->next = NULL;

    return temp;
}

// Removes a node from the front of a list

Node *pop(Node **list) {
    Node *newptr = (*list)->next;
    deleteNode(list);
    return newptr;
}

// Adds a node to the front of a list

void push(Node **list, Node *toAdd) {
    toAdd->next = *list;
    *list = toAdd;
}

// Return a list of pointers in order

void reverseOrder(Node **list) {
    Node* prev = NULL;
    Node* current = *list;
    Node* next;

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

    *list = prev;
}

// Prints the string stored in a single node

void printNode(Node *singleNode) {
    printf("Data : %s", singleNode->data);
}

// Prints an entire linked list. Nodes are printed from first to last

void printLinkedList(Node *linkedList) {
    Node *temp = linkedList;
    
    while(temp!=NULL) {
        printf("Data : %s", temp->data);
        temp = temp->next;
    }
}

When testing the implementation in my driver file, I receive the following error

runtime error: load of null pointer of type 'Node *' (aka 'struct listNode *') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior llist.c:49:19

where line 49 corresponds to toAdd->next = *list in the llist.c file

I am struggling to figure out why this error is occurring as I am calling my push function with the appropriate parameters to an initially empty (NULL) linked list.

driver file (testllist.c) for reference:

#include "llist.h"

int main (int argc, char *argv[]) {
    printf("argc: %d", argc);
    printf("\n\n");

    int num_inputs = argc;
    Node **list = NULL;

    if (argc == 1) {
        printf("No arguments passed.\n");
    } else {
        for (int i = 1; i < num_inputs; i++) {
            printf("String is: %s\n", argv[i]);
            Node *n = newNode(argv[i]);

            printf("String is : %s\n\n", argv[i]);

            push(list, n);

            printLinkedList(*list);
        }


        reverseOrder(list);
        pop(list);
        deleteList(list);
    }
    
    return 0;
}

header file (llist.h) where Node data type and functions are defined

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

// The listNode data type for storing entries in a linked list
typedef struct listNode Node;
struct listNode {
char *data;
Node *next;
};

// Frees all allocated memory associated with the list pointers iteratively
void deleteList(Node **list);

// Frees all allocated memory associated with a single node
void deleteNode(Node **toDelete);

// Allocates memory for a new string and returns a pointer to the memory
Node *newNode(char *string);

// Removes a node from the front of a list and returns a pointer to said node
Node *pop(Node **list);

// Adds a node to the front of a list
void push(Node **list, Node *toAdd);

// Return a list of pointers in order
void reverseOrder(Node **list);

// Prints the string stored in a single node
void printNode(Node *singleNode);

// Prints an entire linked list. Nodes are printed from first to last
void printLinkedList(Node *linkedList);

Solution

  • As you initialized the pointer list as a null pointer

    Node **list = NULL;
    

    then within the function push you may not dereference this pointer

    void push(Node **list, Node *toAdd) {
        toAdd->next = *list;
        *list = toAdd;
    }
    

    And the error message reports this problem.

    You should declare the pointer like

    Node *list = NULL;
    

    and pass it to functions that expect an object of the type Node ** like for example

    push( &list, n );
    

    And it would be much better if the function will be declared like

    int push( Node **list, const char * );
    

    That is it should report whether a new node was successfully pushed or not and the allocation of the new node should be hidden from the user that calls the function.

    Pay attention to that for example the function deleteNode does not make a great sense.

    void deleteList(Node **list) {
        Node* ptr = *list;
        Node* temp;
    
        while(ptr != NULL) {
            free(ptr->data);
            temp = ptr;
            ptr=ptr->next;
            free(temp);
        }
    }
    

    The pointer to the head node of the list is passed by reference. However within the function its value is not being changed. So after exiting the function the pointer to the head node will still have its original value.

    The function can be defined the following way

    void deleteList( Node **list ) 
    {
        while ( *list != NULL )
        {
            Node *ptr = *list;
            *list = ( *list )->next;
            free( ptr->data );
            free( ptr );
        }
    }
    

    The function pop does not check whether the passed pointer to the head node of the list is equal to NULL.

    Node *pop(Node **list) {
        Node *newptr = (*list)->next;
        deleteNode(list);
        return newptr;
    }
    

    Also it returns a pointer to the next node in the list that becomes the current head node. But the returned pointer is not used in main

    pop(list);
    

    Pay attention to that the expression 1 * sizeof( char ) does not make a sense in the expression used as an initializer in this declaration

    temp->data = (char*)malloc(len + 1*sizeof(char));
    

    Either write

    temp->data = (char*)malloc(len + 1);
    

    or like

    temp->data = (char*)malloc( ( len + 1 )*sizeof(char));