Re: [PATCH v5 4/5] pack-redundant: consistent sort method

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jan 10, 2019, 20:05 UTC
Message-ID: <20190110200503.GM4673@szeder.dev>
In-Reply-To: <20190110120142.22271-5-worldhello.net@gmail.com>

On Thu, Jan 10, 2019 at 08:01:41PM +0800, Jiang Xin wrote:

> diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
> index 56591d283f..e9d2586e2e 100644
> --- a/builtin/pack-redundant.c
> +++ b/builtin/pack-redundant.c

Show 23 quoted lines

> @@ -421,16 +422,22 @@ static inline off_t pack_set_bytecount(struct pack_list *pl)
>  	return ret;
>  }
>  
> -static int cmp_pack_list_reverse(const void *a, const void *b)
> +static int cmp_remaining_objects(const void *a, const void *b)
>  {
>  	struct pack_list *pl_a = *((struct pack_list **)a);
>  	struct pack_list *pl_b = *((struct pack_list **)b);
> -	size_t sz_a = pl_a->remaining_objects->size;
> -	size_t sz_b = pl_b->remaining_objects->size;
>  
> -	if (sz_a == sz_b)
> -		return 0;
> -	else if (sz_a < sz_b)
> +	/* if have the same remaining_objects, big pack first */
> +	if (pl_a->remaining_objects->size == pl_b->remaining_objects->size)
> +		if (pl_a->all_objects_size == pl_b->all_objects_size)
> +			return 0;
> +		else if (pl_a->all_objects_size < pl_b->all_objects_size)
> +			return 1;
> +		else
> +			return -1;

My compiler complains about the above nested if statements:

  builtin/pack-redundant.c: In function ‘cmp_remaining_objects’:
  builtin/pack-redundant.c:345:5: error: suggest explicit braces to avoid ambiguous ‘else’ [-Werror=parentheses]
    if (pl_a->remaining_objects->size == pl_b->remaining_objects->size)
       ^
  cc1: all warnings being treated as errors
  Makefile:2302: recipe for target 'builtin/pack-redundant.o' failed

After adding a pair of {} to the outer if statement 't5323-pack-redundant.sh' passed successfully even on macOS (on Travis CI).