[pacman-dev] alpm_list_remove_dups: was - [PATCH] Sort and avoid duplicates in -Sii output

[Dan McGee]

On Wed, Mar 24, 2010 at 1:09 AM, Allan McRae <allan at archlinux.org> wrote:
> On 24/03/10 16:00, Allan McRae wrote:
>>
>> Signed-off-by: Allan McRae<allan at archlinux.org>
>> ---
>>  lib/libalpm/package.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
>> index f1682cb..ed6d71d 100644
>> --- a/lib/libalpm/package.c
>> +++ b/lib/libalpm/package.c
>> @@ -565,7 +565,9 @@ static void find_requiredby(pmpkg_t *pkg, pmdb_t *db,
>> alpm_list_t **reqs)
>>                pmpkg_t *cachepkg = i->data;
>>                if(_alpm_dep_edge(cachepkg, pkg)) {
>>                        const char *cachepkgname = cachepkg->name;
>> -                       *reqs = alpm_list_add(*reqs,
>> strdup(cachepkgname));
>> +                       if(alpm_list_find_str(*reqs, cachepkgname) == 0) {
>> +                               *reqs = alpm_list_add(*reqs,
>> strdup(cachepkgname));
>> +                       }
>>                }
>>        }
>>  }
>> @@ -595,6 +597,7 @@ alpm_list_t SYMEXPORT
>> *alpm_pkg_compute_requiredby(pmpkg_t *pkg)
>>                        for(i = handle->dbs_sync; i; i = i->next) {
>>                                db = i->data;
>>                                find_requiredby(pkg, db,&reqs);
>> +                               reqs = alpm_list_msort(reqs,
>> alpm_list_count(reqs), _alpm_str_cmp);
>>                        }
>>                }
>>        }
>
>
> I was going to use alpm_list_remove_dups instead of testing for a strings
> presence before adding it but I find that function quite impractical...
>
> I wonder if any other frontend is using it and whether we could change it to
> actually removing the duplicates from a list.  i.e. return the same list
> with duplicates removed and their data freed.  Or am I missing some usage
> case where the current functionality is useful?

I'd rather have it not touch the original list. Even if it does modify
the list, freeing items from the list could be disastrous in the case
of packages or something, so that is a no-no. I could see it being a
more useful function if it too took a comparison function and used
that to decide whether two things were equal (passing it along to
alpm_list_find).

One option would be to make the signature look like this:
alpm_list_t *alpm_list_remove_dupes(const alpm_list_t *list,
alpm_list_fn_cmp fn, alpm_list_t **removed);

This would, I think, allow for a variety of use cases. The function
would work similar to what it does now, but use the comparator to
determine whether things are equal and should be purged. In addition,
by returning the removed items from the list, it would allow the
calling site to determine what to do- e.g. free just the resulting
removed list or free the list + items inside.

-Dan