Return to Revisions

Immediate notes:

• Casting malloc (as well as realloc and calloc) is not necessary in C. Leaving them out will not cause warnings or errors and may suppress warnings that would otherwise be helpful. It is necessary in C++, but that's only relevant if you're using a C++ compiler to compile C code. Some reading on Stack Overflow: Should I cast the result of malloc (in C)?
• You are not verifying that malloc has succeeded before dereferencing that pointer by checking it against NULL before use. This can lead to undefined behavior. See linkedlist_init for an example of this.
• In the same function you use the list argument before checking that it is not NULL. You may know that in your current use case a null pointer is never passed in, but can you guarantee that will always be the case?

From a general design standpoint, consider whether you really want to use linked lists extensively in your code, or if you want a dynamic array, more akin to C++'s std::vector. Linked lists have O(n) random access and must perform a dynamic memory allocation on each insertion. A dynamic array can have O(1) access and if designed properly, only O(log n) dynamic memory allocations, and O(1) on de-allocation.

You may wish to watch Bjarne Stroustrup: Why you should avoid Linked Lists.

If you decide you do want to pursue the linked list, then there are some key things to think about.

• You probably want your list struct to hold a pointer to the last in addition to the first node. This makes appending to the end an O(1) operation rather than O(n).
• You probably want to make this an opaque data type so that the user of a list cannot break things like that pointer to the last node. Stack Overflow reading: What defines an opaque type in C, and when are they necessary and/or useful?
• Your node holds a pointer to the data it refers to. Do you want your lists to "own" the data they pointer to, or simply point to it? On freeing a list, should it also free that data?
  • It looks a bit like you were heading in the direction of letting your freeData function pointer decide this.
  • You might implement append and append_copy to handle this issue, with the latter copying data from the value passed in rather than just saving a pointer to it.