Here are some things that may help you improve your code. ## Use `NULL` instead of `0` for pointers The value `0` is an integer, but the value `NULL` is an [implementation-defined null-pointer constant](http://en.cppreference.com/w/c/types/NULL). It is *not* guaranteed to have the value 0. ## Use include guards There should be an include guard in the `.h` file. That is, start the file with: #ifndef LIST_H #define LIST_H // file contents go here #endif // LIST_H ## Simplify your code I'm not sure why the `x_upcast` and `data_upcast` code exists. Maybe the intent was to more cleanly separate the `data` type (an `int` here) from the rest of the code. However, consider that it could instead be written like this: static struct Link *x_upcast(struct X *const x) { return (struct Link *)x; } Better though, in my opinion, would be to eliminate it entirely. The single place it's used is in `ListForEach`: for(x = list->head.next; (next_x = x->next); x = next_x) action(&(x_upcast)(x)->data); This could be more clearly written as: for(x = list->head.next; (next_x = x->next); x = next_x) action(&((struct Link *)x)->data); This also brings us to the next suggestion. ## Use the appropriate data type The code, as posted, appears to treat pointers to the data value and pointers to a `Link` identically. This is a problem because it misleads the reader. For example, the `get_link` code creates and partially initializes a `Link` but claims to be returning an `int *`. This would be much more clear if, instead, the code were to actually return a `struct Link *`. In other words, the interface should *guide correct usage* rather than *encourage incorrect usage*. As an example, this code compiles just fine: int n = 99; ListPush(&list, &n); However this is a runtime disaster waiting to happen. We would probably prefer that it not even compile because what `ListPush` actually requires is a pointer to the data member of an already created `struct Link`. The next suggestion addresses this problem. ## Rethink your interface If `LinkPush` really requires a `List` and a `Link`, let's declare it that way. Instead of this: void ListPush(struct List *const list, int *const add); use this: void ListPush(struct List *const list, struct Link *const newnode); Now compiler will actually assist and point out bad usage like the code mentioned previously. This eliminates the type name `X` and also requires some redefinitions of other things such as `List` and `Action` which now look like this: struct List { struct Link head, tail; }; typedef void (*Action)(struct Link *const); ## Use better names The type name `List` is good, but the type name `X` is not. The first name explains something about what the variable means within the context of the code, but the latter is opaque and non-descriptive. ## Better describe the responsibilities of the data structure It is quite important to note that this implementation of a linked list assumes that some other entity is creating (and presumably deleting) its nodes. It gives the user some flexibility, as it would allow the use of statically or dynamically allocated memory, but it's worth explicitly mentioning to the user of the code in a comment.