On Thu, Jan 10, 2019 at 08:01:39PM +0800, Jiang Xin wrote:
> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index cf9a9aabd4..3655cc7dc6 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c

> @@ -446,49 +484,37 @@ static void minimize(struct pack_list **min)
>               pl = pl->next;
>       }
>  
> +     *min = unique;
> +
>       /* return if there are no objects missing from the unique set */
>       if (missing->size == 0) {
> -             *min = unique;
>               free(missing);
>               return;
>       }
>  
> -     /* find the permutations which contain all missing objects */
> -     for (n = 1; n <= pack_list_size(non_unique) && !perm_ok; n++) {
> -             perm_all = perm = get_permutations(non_unique, n);
> -             while (perm) {
> -                     if (is_superset(perm->pl, missing)) {
> -                             new_perm = xmalloc(sizeof(struct pll));
> -                             memcpy(new_perm, perm, sizeof(struct pll));
> -                             new_perm->next = perm_ok;
> -                             perm_ok = new_perm;
> -                     }
> -                     perm = perm->next;
> -             }
> -             if (perm_ok)
> -                     break;
> -             pll_free(perm_all);
> -     }

Please make sure that all commits in the patch series can be build
cleanly without any warnings (with '-Werror' or preferably with 'make
DEVELOPER=1') and pass the test suite.  This is important, because
unbuildable commits will cause trouble later on, when e.g. 'git
bisect' happens to pick such a commit.

In this case, the removal of the above loop removes all callsites of
the static functions get_permutations(), is_superset(), and
pll_free(), resulting the following compiler error:

  builtin/pack-redundant.c: At top level:
  builtin/pack-redundant.c:289:13: error: ‘pll_free’ defined but not used 
[-Werror=unused-function]
   static void pll_free(struct pll *l)
               ^
  builtin/pack-redundant.c:309:21: error: ‘get_permutations’ defined but not 
used [-Werror=unused-function]
   static struct pll * get_permutations(struct pack_list *list, int n)
                       ^
  builtin/pack-redundant.c:343:12: error: ‘is_superset’ defined but not used 
[-Werror=unused-function]
   static int is_superset(struct pack_list *pl, struct llist *list)
              ^

I see that the last patch in this series removes those three
unused functions, but that patch should be squashed into this one to
keep Git buildable with '-Werror' or DEVELOPER=1.

Furthermore, after building this patch (without '-Werror'), several
tests in 't5323-pack-redundant.sh' fail.  To avoid the test failure I
think the fourth patch ensuring a consistent sort order should be
squashed in as well.


Reply via email to