Re: [PATCH 4/4] ALLOC_GROW: avoid -Wsign-compare warnings

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 22, 2017, 04:20 UTC
Message-ID: <xmqqd16jl5yb.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <c97784ce-d85d-2b7a-4eb7-d4043dc1a0b7@ramsayjones.plus.com>

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Show 10 quoted lines

> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>  builtin/pack-objects.c |  4 ++--
>  config.c               |  2 +-
>  diff.c                 |  2 +-
>  line-log.c             | 18 +++++++++---------
>  line-log.h             |  2 +-
>  revision.c             |  2 +-
>  tree-walk.c            |  3 +--
>  7 files changed, 16 insertions(+), 17 deletions(-)

Thanks.

I did not spot any questionable conversion (e.g. "something that used to be signed was because it wanted to store -1 as a sentinel" would be broken if we just change that to unsigned) by going over the places these variables and fields are actually used.

A review of a patch like this involves reading through 10x more lines than we see in the above diffstat, and producing it would most likely have taken the same amount of effort, at least. Very much appreciated.

Show 12 quoted lines

> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index a57b4f058..a6ee653bf 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -2563,8 +2563,8 @@ struct in_pack_object {
>  };
>  
>  struct in_pack {
> -	int alloc;
> -	int nr;
> +	unsigned int alloc;
> +	unsigned int  nr;

This is a bit questionable ;-) but it is something I can locally tweak easily.