Generic linked list

Question

I provided an answer to a question on SO about a generic linked list. I just had the curiosity of having my code peer-reviewed as I wrote the code specifically for the answer.

That is, I have not implemented a generic linked list before, and would like to see if there are minor issues with the code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct node {
 void *data;
 node *next;
} node;
int lst_nodeAdd(
 node **head,
 node **tail,
 const void *data,
 size_t szData);
void lst_nodePrint(
 node *head,
 void(*print)(const void *));
void lst_nodeFree(node *head);
/* PRINTING FUNCTIONS */
void print_int(const void *a);
void print_string(const void *str);
int main(void)
{
 const char *str[] = {
 "0x0001",
 "0x0002",
 "0x0003",
 };
 // head & tail
 node *head = NULL;
 node *tail = NULL;
 // List of strings
 lst_nodeAdd(&head, &tail, str[0], strlen(str[0]) + 1);
 lst_nodeAdd(&head, &tail, str[1], strlen(str[1]) + 1);
 lst_nodeAdd(&head, &tail, str[2], strlen(str[2]) + 1);
 lst_nodePrint(head, print_string);
 lst_nodeFree(head);
 head = NULL;
 tail = NULL;
 //....................................................
 // List of ints
 int int_array[] = {
 0,
 1,
 2,
 };
 lst_nodeAdd(&head, &tail, &int_array[0], sizeof(int));
 lst_nodeAdd(&head, &tail, &int_array[1], sizeof(int));
 lst_nodeAdd(&head, &tail, &int_array[2], sizeof(int));
 lst_nodePrint(head, print_int);
 lst_nodeFree(head);
 head = NULL;
 tail = NULL;
 system("PAUSE");
 return 0;
}
int lst_nodeAdd(
 node **head,
 node **tail,
 const void *data,
 size_t szData)
{
 void *tmp;
 tmp = malloc(sizeof(node));
 if (!tmp)
 {
 return 0;
 }
 ((node *)tmp)->next = NULL;
 ((node *)tmp)->data = malloc(szData);
 if (!((node *)tmp)->data)
 {
 free(tmp);
 return 0;
 }
 memcpy(((node *)tmp)->data, data, szData);
 if (!*head)
 {
 *head = (node *)tmp;
 }
 else
 {
 (*tail)->next = (node *)tmp;
 }
 *tail = (node *)tmp;
 return 1;
}
void lst_nodePrint(
 node *head,
 void(*print)(const void *))
{
 while (head)
 {
 print(head->data);
 head = head->next;
 }
}
void lst_nodeFree(node *head)
{
 node *tmp = head;
 while (head)
 {
 head = head->next;
 free(tmp->data);
 free(tmp);
 tmp = head;
 }
}
void print_int(const void *a)
{
 printf("%d\n", *(const int *)a);
}
void print_string(const void *str)
{
 puts((const char *)str);
}

Answer 1

Comment

"implemented a generic linked list', this codes seems to be focused on adding to the list variant sizes of data - which in good for strings. I would find this a bit error prone or tedious and prefer to provide the size at link-list head creation time, rather than provided it each time with lst_nodeAdd().

Major

Invalid standard C code

node is not defined yet used in node *next;

typedef struct node {
 void *data;
 // node *next;
 struct node *next;
} node;

Minor

Odd use of void *

Every reason to use node *tmp and not void *tmp with all the unneeded casting. I suspect this is another reflection of compiling to C++ rather than C.

// void *tmp;
// if (!*head) {
// *head = (node *)tmp;
// } else {
// (*tail)->next = (node *)tmp;
// } *tail = (node *)tmp;
node *tmp;
if (!*head) {
 *head = tmp;
} else {
 (*tail)->next = tmp;
}
*tail = tmp;

Allocate to the size of the de-referenced type.

With above change, size to the variable, not its type. Easier to code, review and maintain.

// tmp = malloc(sizeof(node));
tmp = malloc(sizeof *tmp);

The size of user data may be 0

Although strange, there is no reason to not support a size of 0. Just need to modify the allocation check as a return of NULL from malloc(0) is OK.

// if (!((node *) tmp)->data) {
if (!((node *) tmp)->data && szData > 0) {

Use const

For functions that do not change the referenced data, use const to better convey code's intent and applicability.

// void lst_nodePrint(node *head, void (*print)(const void *)) {
void lst_nodePrint(const node *head, void (*print)(const void *)) {

Code looks wrong

 // OK, yet perhaps ...
 node *tmp = head;
 while (head) {
 head = head->next;
 free(tmp->data);
 free(tmp);
 tmp = head;
 }
 // ... this instead
 while (head) {
 node *next = head->next;
 free(head->data);
 free(head);
 head = next;
 }

Missing header

Make a separate header file like lst.h to show what goes in there and not in the companion lst.c

Implementation vs test separation

Move these test declarations and functions to another file or to the bottom of the post to show a clear demarcation of test code.

void print_int(const void *a);
void print_string(const void *str);
int main(void) { ...

Other

Good code that avoids leak when only one allocation worked.

if (!((node *) tmp)->data) {
 free(tmp);
 return 0;
}

Perhaps more later.