Return to Answer

node_t *retptr = NULL;
retptr = (node_t *)malloc( sizeof(node_t) );

Why not just node_t retptr = malloc( sizeof(node_t) );

In free_node() setting n = NULL does nothing. In fact as a reviewer it "pricks my senses" and makes me wonder if you understand pointers - so I'd spend more time deeply reviewing because I'm now suspicious.

int cmp_fn() is using assert to check things that really don't belong in an assert. It should return "not match" in most of those conditions.

free_list() Why on earth would you make this recursive?

Nothing stops me inserting even after the list is full. Therefore return head->len == MAX_LIST_LEN; should really be return head->len >= MAX_LIST_LEN; otherwise it is misleading.

If you really want to remove from the front and the end, consider a doubly linked list? (Otherwise what is the real reason of keeping the end pointer at all?)

insert_at, remove_at etc can all be simplified by using find_at to locate the correct node (which may be the one before the one you need to play with. Why isn't insert_atfront just a call to insert_at(head, 0, anode)? I'm sure there are other similar cases.

I stopped here - in general my worst complain is too much assert - if you compile not in DEBUG mode behavior will be different (e.g. no checking for NULLs)

node_t *retptr = NULL;
retptr = (node_t *)malloc( sizeof(node_t) );

Why not just node_t retptr = malloc( sizeof(node_t) );

In free_node() setting n = NULL does nothing because n is just local to the function. Whatever calls it still has a pointer to memory that has just been freed. In fact as a reviewer it "pricks my senses" and makes me wonder if you understand pointers - so I'd spend more time deeply reviewing because I'm now suspicious.

int cmp_fn() is using assert to check things that really don't belong in an assert. It should return "not match" in most of those conditions.

free_list() Why on earth would you make this recursive?

Nothing stops me inserting even after the list is full. Therefore return head->len == MAX_LIST_LEN; should really be return head->len >= MAX_LIST_LEN; otherwise it is misleading.

If you really want to remove from the front and the end, consider a doubly linked list? (Otherwise what is the real reason of keeping the end pointer at all?)

insert_at, remove_at etc can all be simplified by using find_at to locate the correct node (which may be the one before the one you need to play with. Why isn't insert_atfront just a call to insert_at(head, 0, anode)? I'm sure there are other similar cases.

I stopped here - in general my worst complain is too much assert - if you compile not in DEBUG mode behavior will be different (e.g. no checking for NULLs)

node_t *retptr = NULL;
retptr = (node_t *)malloc( sizeof(node_t) );

Why not just node_t retptr = malloc( sizeof(node_t) );

In free_node() setting n = NULL does nothing. In fact as a reviewer it "pricks my senses" and makes me wonder if you understand pointers - so I'd spend more time deeply reviewing because I'm now suspicious.

int cmp_fn() is using assert to check things that really don't belong in an assert. It should return "not match" in most of those conditions.

free_list() Why on earth would you make this recursive?

Nothing stops me inserting even after the list is full. Therefore return head->len == MAX_LIST_LEN; should really be return head->len >= MAX_LIST_LEN; otherwise it is misleading.

If you really want to remove from the front and the end, consider a doubly linked list? (Otherwise what is the real reason of keeping the end pointer at all?)

insert_at, remove_at etc can all be simplified by using find_at to locate the correct node (which may be the one before the one you need to play with.

I stopped here - in general my worst complain is too much assert - if you compile not in DEBUG mode behavior will be different (e.g. no checking for NULLs)

node_t *retptr = NULL;
retptr = (node_t *)malloc( sizeof(node_t) );

Why not just node_t retptr = malloc( sizeof(node_t) );

In free_node() setting n = NULL does nothing. In fact as a reviewer it "pricks my senses" and makes me wonder if you understand pointers - so I'd spend more time deeply reviewing because I'm now suspicious.

int cmp_fn() is using assert to check things that really don't belong in an assert. It should return "not match" in most of those conditions.

free_list() Why on earth would you make this recursive?

Nothing stops me inserting even after the list is full. Therefore return head->len == MAX_LIST_LEN; should really be return head->len >= MAX_LIST_LEN; otherwise it is misleading.

If you really want to remove from the front and the end, consider a doubly linked list? (Otherwise what is the real reason of keeping the end pointer at all?)

insert_at, remove_at etc can all be simplified by using find_at to locate the correct node (which may be the one before the one you need to play with. Why isn't insert_atfront just a call to insert_at(head, 0, anode)? I'm sure there are other similar cases.

I stopped here - in general my worst complain is too much assert - if you compile not in DEBUG mode behavior will be different (e.g. no checking for NULLs)

node_t *retptr = NULL;
retptr = (node_t *)malloc( sizeof(node_t) );

Why not just node_t retptr = malloc( sizeof(node_t) );

In free_node() setting n = NULL does nothing. In fact as a reviewer it "pricks my senses" and makes me wonder if you understand pointers - so I'd spend more time deeply reviewing because I'm now suspicious.

int cmp_fn() is using assert to check things that really don't belong in an assert. It should return "not match" in most of those conditions.

free_list() Why on earth would you make this recursive?

I stopped here - in general my worst complain is too much assert - if you compile not in DEBUG mode behavior will be different (e.g. no checking for NULLs)

node_t *retptr = NULL;
retptr = (node_t *)malloc( sizeof(node_t) );

Why not just node_t retptr = malloc( sizeof(node_t) );

In free_node() setting n = NULL does nothing. In fact as a reviewer it "pricks my senses" and makes me wonder if you understand pointers - so I'd spend more time deeply reviewing because I'm now suspicious.

int cmp_fn() is using assert to check things that really don't belong in an assert. It should return "not match" in most of those conditions.

free_list() Why on earth would you make this recursive?

Nothing stops me inserting even after the list is full. Therefore return head->len == MAX_LIST_LEN; should really be return head->len >= MAX_LIST_LEN; otherwise it is misleading.

If you really want to remove from the front and the end, consider a doubly linked list? (Otherwise what is the real reason of keeping the end pointer at all?)

insert_at, remove_at etc can all be simplified by using find_at to locate the correct node (which may be the one before the one you need to play with.

I stopped here - in general my worst complain is too much assert - if you compile not in DEBUG mode behavior will be different (e.g. no checking for NULLs)