On Fri, Jun 24, 2011 at 6:22 PM, Allan McRae wrote:

This saves replicating the list of files in a package that is being removed which can be quite large.

Signed-off-by: Allan McRae ---

The tradeoff here is a slightly less readable loop, but duplicating that list just struck me as wasteful.

I agree with the idea; I agree the implementation is a tad crazy. What if we added an alpm_list_prev() function instead? This wouldn't directly mirror alpm_list_next() due to the whole "quick last item" bit, so instead it would be something like: alpm_list_t *alpm_list_prev(alpm_list_t *list, alpm_list_t *item); and the ickyness would be wrapped up in here, special casing if list == item. Then this patch would be a one-liner change to the loop to something like the following: for(lp = alpm_list_last(files); lp; lp = alpm_list_prev(files, lp)) {

 lib/libalpm/remove.c |   41 +++++++++++++++++++++++------------------  1 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index cc6daea..2d75a84 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -285,8 +285,7 @@ static void unlink_file(pmhandle_t *handle, pmpkg_t *info, const char *filename,  int _alpm_upgraderemove_package(pmhandle_t *handle,                pmpkg_t *oldpkg, pmpkg_t *newpkg)  { -       alpm_list_t *skip_remove, *b; -       alpm_list_t *newfiles, *lp; +       alpm_list_t *skip_remove, *b, *lp;        size_t filenum=0;        alpm_list_t *files = alpm_pkg_get_files(oldpkg);        const char *pkgname = alpm_pkg_get_name(oldpkg); @@ -329,11 +328,15 @@ int _alpm_upgraderemove_package(pmhandle_t *handle,        _alpm_log(handle, PM_LOG_DEBUG, "removing %ld files\n", (unsigned long)filenum);

       /* iterate through the list backwards, unlinking files */ -       newfiles = alpm_list_reverse(files); -       for(lp = newfiles; lp; lp = alpm_list_next(lp)) { -               unlink_file(handle, oldpkg, lp->data, skip_remove, 0); +       if(files) { +               alpm_list_t *last = alpm_list_last(files); +               lp = last; +               do { +                       unlink_file(handle, oldpkg, lp->data, skip_remove, 0); +                       lp = lp->prev; +               } while (lp != last);        } -       alpm_list_free(newfiles); +        FREELIST(skip_remove);

 db: @@ -390,7 +393,6 @@ int _alpm_remove_packages(pmhandle_t *handle)

               if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) {                        alpm_list_t *files = alpm_pkg_get_files(info); -                       alpm_list_t *newfiles;                        size_t filenum=0;

                       for(lp = files; lp; lp = lp->next) { @@ -409,18 +411,21 @@ int _alpm_remove_packages(pmhandle_t *handle)                                        pkg_count, (pkg_count - targcount + 1));

                       /* iterate through the list backwards, unlinking files */ -                       newfiles = alpm_list_reverse(files); -                       for(lp = newfiles; lp; lp = alpm_list_next(lp)) { -                               int percent; -                               unlink_file(handle, info, lp->data, NULL, trans->flags & PM_TRANS_FLAG_NOSAVE); - -                               /* update progress bar after each file */ -                               percent = (position * 100) / filenum; -                               PROGRESS(trans, PM_TRANS_PROGRESS_REMOVE_START, info->name, -                                               percent, pkg_count, (pkg_count - targcount + 1)); -                               position++; +                       if(files) { +                               alpm_list_t *last = alpm_list_last(files); +                               lp = last; +                               do { +                                       int percent; +                                       unlink_file(handle, info, lp->data, NULL, trans->flags & PM_TRANS_FLAG_NOSAVE); + +                                       /* update progress bar after each file */ +                                       percent = (position * 100) / filenum; +                                       PROGRESS(trans, PM_TRANS_PROGRESS_REMOVE_START, info->name, +                                                       percent, pkg_count, (pkg_count - targcount + 1)); +                                       position++; +                                       lp = lp->prev; +                               } while (lp != last);                        } -                       alpm_list_free(newfiles);                }

               /* set progress to 100% after we finish unlinking files */ -- 1.7.5.4