Simple but (hopefully) safe C nodes for data structures

Question

Nodes will be for simply linked lists, circular lists, maybe later on extending them to work with graphs. I'm trying to be as defensive as possible, will this leak memory? It runs perfectly I'm just concerned about coding style and best practices. Especially regarding the allocation I'm not 100% convinced of my solution. Is it okay to allocate memory for the next component inside the constructor?

// I would have preferred to declare the typedef together with the struct, 
// but this doesn't work.
typedef struct node Node;   


struct node {
    int value;
    Node *next;
};


Node *create_node(int value) {

    Node *result = malloc(sizeof(Node));
    // How necessary are these checks? Under which circumstances 
    // could this return NULL?
    if (result == NULL) {           
        return NULL;
    }

    result->next = malloc(sizeof(Node));
    if (result->next == NULL) {
        free(result);
        return NULL;
    }

    result->value = value;

    return result;
}


void destroy_node(Node *node) {
    if (node != NULL) {
        //This seems necessary. Or is it not? Could I delete the parent 
        // node and keep the children alive?
        destroy_node(node->next);   
        free(node);
    }
}

int main() {

    Node *first = create_node(42);
    printf("%d\n", first->value);

    Node *second = create_node(45);
    first->next = second;
    printf("%d\n", first->next->value);

    destroy_node(first);

    return 0;
}

Answer 1

It's very misleading that destroy_node (singular) actually recursively frees all nodes in the chain.

It's very weird that create_node allocates two nodes rather than one. In fact, you do have a memory leak in your main():

first->next = second;

There, you have just overwritten the 42 node's next pointer without having freed it.

---

To answer this question…

// How necessary are these checks? Under which circumstances 
// could this return NULL?
if (result == NULL) {           
    return NULL;
}

If malloc() fails (because no more memory is available), it will return NULL. Yes, it is good practice to check whether malloc() failed. (In practice, if you are so low on memory that you can't even allocate a single linked-list node, it's very difficult to recover gracefully. Your program is going to die, and there's not much you can do about it.)