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);
}
}
To me, I'd say
n
andn.next
are both typenode
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 struct
s 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.