On 26.02.2012 09:25, Stefan Götz wrote:
To separate memory management from list management, I would find it a cleaner API to not pass in a free function pointer but to return a void* pointer. If ptr was found in the list and removed, the function should then return ptr itself. If ptr was not found in the list, the function should return NULL. In any case, the caller can free the memory for ptr (if they intend to do so) by calling free(hip_ll_del_by_ptr(list, ptr)); This separates concerns and is still very easy to use.
I'd rather vote for someone to pick up and finish your existing linked list branch instead (one day), so we have a single implementation for both hipd and hipfw.
I agree that passing the free function every time is stupid. Note though that we could go the other extreme and store alloc/free function pointers in the list head instead, to avoid this duplication altogether. But ultimately your solution would wreak the least havoc, because there is nothing unexpected going on behind the scenes (easier to comprehend for new contributors) and we're unlikely to refactor this aspect.
+{ + struct hip_ll_node *curr; + struct hip_ll_node *tmp; + + /* match first list node */ + if (linkedlist != NULL&& linkedlist->element_count> 0the check for linkedlist != NULL should also cover the second part of this function. I'd suggest adding an assertion for it.
Passing NULL to free()-like functions is usually a no-op (by convention), so I disagree.