[pacman-dev] alpm_list_remove is buggy

[Nagy Gabor]

> On Fri, Oct 26, 2007 at 08:46:53PM +0200, Nagy Gabor wrote:
> > Hi!
> > That function seems totally odd.
> > /* TODO I modified this to remove ALL matching items.  Do we need a
> > remove_first? */ <- this is partly true:
> > *data contains only the last found "i->data", and the function
> > doesn't return with an alpm_list. MEMLEAK!!
> > 
> > Bye, ngaba
> > 
> > PS: I don't create a patch, because I dunno want do you want: remove
> > the matching _list_ or remove the first matching _item_'s data. My
> > vote: 2nd. (Because then I don't have to resubmit my -Ru patch ;-)
> > 
> > 
> 
> That doesn't look right indeed.
> I also didn't notice this behavior of alpm_remove directly, and I
> found it odd when I discovered that it removed all matching items.
> Not sure why, but I would prefer go the other way around : having the
> default remove act as remove_first (ie the old behavior), and maybe
> add a new remove_all functions (without the memleak :P) if it's
> really needed.
> 
> But then we need to know where this is really needed, and why that
> remove function was edited to remove all matching items.

I emphasise, that the definition of the functions says, that it will
return with an alpm_list (remove == cut here). But it doesn't do that,
it returns with a pointer to data (and it frees the alpm_list header).
All parts of the libalpm code assumes that it will return with a pointer
to the data.
I also prefer the old behavior, because:
1. That is faster.
2. Can anyone show me a code-part where the new behavior is needed?

Bye, ngaba