Search code examples
cstructlinked-listsingly-linked-listfunction-definition

linked list problem when displaying the list in C


having segmentation error while trying to access nodes

i can create new nodes with my add function after function executes i cant access my nodes. i think they deallocated in memory but i couldnt figure it out.

#include <stdio.h>
#include <stdlib.h>
struct node
{
    int data;
    struct node *nextNode;
};
struct node *head;
void add(int data)
{
    

    struct node *new = (struct node *)malloc(sizeof(struct node));
    new->data = data;
    new->nextNode = NULL;
    struct node *temp1;
    temp1 = head;
    
    while (temp1 != NULL)
    {
        temp1 = temp1->nextNode;
    }

    temp1 = new;
    printf("\nValue of temp1:%d\nValue of new: %d\n",temp1,new);
    printf("\nData of temp1:%d\nData of new:%d\n",temp1->data,new->data);
}
void printList()
{
    int i = 1;
    struct node *tempP;
    tempP = head;
    while (tempP != NULL)
    {
        printf("\nData of %dth element is : %d\n", i, tempP->data);
        tempP = tempP->nextNode;
        i++;
    }
}

void main()
{
    head = (struct node *)malloc(sizeof(struct node));
    head->data = 10;
    head->nextNode = NULL;
    add(20);
    add(30);
    add(40);
    printList();
   
}


Solution

  • This code snippet within the function add

    struct node *temp1;
    temp1 = head;
    
    while (temp1 != NULL)
    {
        temp1 = temp1->nextNode;
    }
    
    temp1 = new;
    

    is wrong. Within it there is changed the local variable temp1. It is not linked with the list.

    Also using the conversion specifier %d to output a pointer invokes undefined behavior. You should use conversion specifier %p.

    Using your approach to the function definition you could write instead.

    void add(int data)
    {
        struct node *new = malloc( sizeof( *new ) );
        new->data = data;
        new->nextNode = NULL;
    
        if ( head == NULL )
        {
            head = new;
        }
        else
        {
            struct node *temp1 = head;
        
            while ( temp1->nextNode != NULL)
            {
                temp1 = temp1->nextNode;
            }
    
            temp1->nextNode = new;
        }
    
        printf("\nValue of temp1->nextNode:%p\nValue of new: %p\n", 
               ( void * )temp1->nextNode, ( void * )new);
        printf("\nData of temp1->nectNode:%d\nData of new:%d\n",
                temp1->nextNode->data,new->data);
    }
    

    Pay attention to that it is a bad design when functions depend on a global variable as in your case where the functions depend on the global variable head.

    And it is also a bad idea when the first node is added to the list bypassing the function add.

    And you need check whether a node was successfully allocated.

    Also according to the C Standard the function main without parameters shall be declared like

    int main( void )
    

    As for me I would declare the pointer to the head node in main like

    int main( void )
    {
        struct node *head = NULL;
        // ...
    

    And the function add will look like

    int add( struct node **head, int data )
    {
        struct node *new_node = malloc( sizeof( *new_node ) );
        int success = new_node != NULL;
    
        if ( success )
        {
            new_node->data = data;
            new_node->nextNode = NULL;
    
            while ( *head != NULL ) head = &( *head )->nextNode;
    
            *head = new_node;
        }
    
        return success;
    }
    

    and called like

    struct node *head = NULL;
    
    add( &head, 10 );
    add( &head, 20 );
    add( &head, 30 );
    add( &head, 40 );
    

    In turn the function printList can look like

    void printList( const struct node *head )
    {
        for ( size_t i = 1; head != NULL; head = head->nextNode )
        {
            printf( "Data of %zuth element is : %d\n", i++, head->data);
        }
    }
    

    And you need at least to write one more function that will free all the allocated memory.