Why is this linked-list initialization not working?

Question

I'm trying to initialize (or create) a linked-list with an empty node pointing to NULL, but it's returning an error and I don't know why. Can somebody help me?

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

struct node {
    int times;
    char name[100];
    char number[100];  
    struct node* next;
};

typedef struct node* node;

void mklist(node* n) {
    (*n)->times = 0;
    strcpy((*n)->name, "null");
    strcpy((*n)->number, "null");
    (*n)->next = (node)NULL;
}

int main(void) {
    node n;
    mklist(&n);
    return 0;
}

Don't hide pointers behind a typedef...very confusing and error-prone. — Stephen Docy, May 23 '18 at 22:39
[is it a good idea to typedef pointers](https://stackoverflow.com/questions/750178/is-it-a-good-idea-to-typedef-pointers) — Barmar, May 23 '18 at 22:49

Stephen Docy · Answer 1 · 2018-05-23T22:50:06.080

1

So node is actually a pointer to a struct node, very confusing

typedef struct node* node;

In main() you declare a pointer and pass a pointer to a pointer to mklist()

node n;
mklist(&n);

In mklist(), n is actually a pointer to a pointer to the struct, so derefencing it, you get a pointer to a struct

void mklist(node* n){
    (*n)->times=0;

but nowhere in your code have you allocated memory for an actual struct.

The most straightforward fix with the way your code is currently is to add a malloc()

void mklist(node* n) {
    *n = malloc(sizeof(*(*n)));
    // check for malloc() failure

    (*n)->times = 0;
    strcpy((*n)->name, "null");
    strcpy((*n)->number, "null");
    (*n)->next = (node)NULL;
}

edited May 23 '18 at 22:50

answered May 23 '18 at 22:43

Stephen Docy

4,738
7
18
31

I have a lot of trouble with dynamic allocation, and I still don't understand it pretty well. Should I allocate the struct in the mklist() function? – Rafael Santos May 23 '18 at 22:50
You should allocate memory whenever you are adding and filling in a new node in your list. Think of `malloc()` as actually building the struct for you to store information in. `malloc()` within a function is perfectly fine, the memory is yours until you explicitly free it. As long as you return a pointer to the new memory or correctly update existing pointers to point to it, you will maintain access to the memory. – Stephen Docy May 23 '18 at 22:54
Memory allocation is a tricky subject to grasp at first, so stick with it. Eventually, data structures like linked lists will become second-nature if you keep playing with them. – Stephen Docy May 23 '18 at 22:56
Thanks a lot, it solved the problem. I will study more about pointers and allocation so it won't be a nightmare anymore haha. – Rafael Santos May 23 '18 at 23:03

score 0 · Answer 2 · answered May 23 '18 at 23:13

You can do this easily in the global scope:

// nil = &nilObj, which means nil->next == nil.
// 
// This way, there's no checking for both 'nil' and 'NULL'!
// 
// As a consequence of this last point, you can make passing 'NULL'
// to most list functions invalid: it just means the user didn't
// use mklist() on every list they needed to manually work with.
static struct node nilObj = { 0, "null", "null", &nilObj };
node nil = &nilObj;

void mklist(node *n)
{
    *n = nil;
}

As Stephen Docy mentioned, using typedef T *Tname; is generally a bad idea as it hides the fact that you're using a pointer, which can be confusing when you use Tname *n as (*n)->foo (I'd expect to use it as n->foo honestly). Some APIs do this, but they do it in a way that expresses that the variable is a pointer to an object rather than an object: instead of Node, something like NodeRef or NodePtr is used, signifying in the name that it's a pointer to a node, not a node. Apple's Core Foundation API uses names like this (e.g. CFStringRef). I highly suggest adopting a convention similar to this henceforth. The above code I posted might then look something like this:

static struct node nilObj = { 0, "null", "null", &nilObj };
nodeRef nil = &nilObj;

void mklist(nodeRef *n)
{
    *n = nil;
}

Why is this linked-list initialization not working?

2 Answers2