[pacman-dev] [PATCH] Add remove counterparts to alpm_option_add_* functions

[Nagy Gabor]

> Quick first thought here is these functions should not be void return
> type, but int so they can return whether they were successful or not.
> Of course, that provokes a second thought that notices that the list
> remove function has a rather odd function signature.

Well, I don't know. alpm_list_remove is one of our most reliable
functions, I cannot see any reason why it should fail (for example
alpm_list_add is not so reliable, because it must allocate some memory
for the new node header).
So if I understood correctly, you want to indicate whether the
to-be-removed element was found or not. I don't know if this is needed
or not, because as I said above we can guarantee, that if the
to-be-removed element exists, we remove it (one of them); and user
probably wants to remove an existing element.
But returning with your preferred integer doesn't hurt anything, so /me
shrugs.

> It looks like you could pass in a data pointer (set your vdata ptr to
> null first to be safe), and check after the remove call if something
> is in this pointer. If there was something, you actually have to free
> it (because of memory leak issues, which I just realized would
> happen). Finally, return true (1) if there was data returned, and
> false (0) if data was not found.

This data pointer is vdata in the patch (that's why I also said:
memleak).
 
> Thoughts from anyone else also welcome.
> 
> -Dan
> 

Bye