[hipl-dev] Re: [Merge] lp:~hipl-core/hipl/libhip into lp:hipl

  • From: Christof Mroz <christof.mroz@xxxxxxxxxxxxxx>
  • To: hipl-dev@xxxxxxxxxxxxx
  • Date: Sun, 26 Feb 2012 12:50:33 +0100

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>  0

the 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.

Other related posts: