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