On Wed, Mar 24, 2010 at 1:09 AM, Allan McRae <allan@archlinux.org> wrote:

On 24/03/10 16:00, Allan McRae wrote:

...

Signed-off-by: Allan McRae<allan@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