Search code examples
cnodes

Does a node's behaviour change when it is a member of a struct?


I'm trying to learn how to use linked lists, so I've written myself a function to recursively go through a linked list and print a word stored in each node, but it's only printing the penultimate item and then repeating indefinitely. I've debugged this and I can see it's because the last node will satisfy n.next != NULL, so I wanted to change the condition to n != NULL to avoid this, but I get the error message: error: invalid operands to binary expression ('node' (aka 'struct node') and 'void *'). I've tried to search the error message on Google and SO but I can't explain why n.next != NULL compiles nicely but n != NULL doesn't. To me, I'd say n and n.next are both type node, but presumably my intuition is deceiving me somehow. Is it because n.next is a struct member that it's behavior changes, or am I on the wrong track?

I include the code below (function in question is at the bottom):

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

typedef struct node
{
    char word[20];
    struct node *next;
}
node;

void print (node n);

node *table[1];

int main(void)
{
    // TODO
    char word[20];

    FILE *file = fopen("list", "r");
    node *first = malloc(sizeof(node));
    table[0] = first;

    if (file != NULL)
    {
        while (fscanf(file, "%s", word) != EOF)
        {
            node *entry = malloc(sizeof(node));
            if (entry != NULL)
            {
                strcpy (entry->word, word);
                entry->next = first->next;
                first->next = entry;
            }
        }
    }
    
    print(*first);
}

void print (node n)
{
    while(n != NULL)
    {
        printf("%s\n", n.word);
        print(*n.next);
    }
}

Solution

  • To me, I'd say n and n.next are both type node

    Not so; n is a node, but n.next is type node *, i.e. a pointer to a node. Pointers can be null but structs cannot.

    Thus the object passed to print is guaranteed valid. (If first were a null pointer then print(*first) would already have crashed, or "caused undefined behavior", before you even entered print.)

    It's also not necessary to have a loop in print, since the recursion handles the list traversal. Indeed, if you try to keep the loop as it is, it's an infinite loop, because nothing in the body modifies the value of n.

    I would write:

    void print (node n)
    {
        printf("%s\n", n.word);
        if (n.next != NULL)
            print(*n.next);
    }
    

    However this approach is not really idiomatic, and it's also not very efficient, since passing structs by value tends to involve unnecessary copying and stack usage. It'd be more common, as dbush suggests, to have a version that takes pointers:

    void print(const node *np)
    {
        if (np)
        {
            printf("%s\n", np->word);
            print(np->next);
        }
    }
    

    which you then call as print(first);.

    A next good exercise would be to try to write a version of print that doesn't use recursion, since that will allow you to handle very long lists that might exceed your stack size.