Re: [PATCH v2 1/1] add: use unsigned type for collection of bits

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 29, 2024, 19:52 UTC
Message-ID: <xmqqttlrdp5f.fsf@gitster.g>
In-Reply-To: <20240229194444.8499-2-giganteeugenio2@gmail.com>

Eugenio Gigante <giganteeugenio2@gmail.com> writes:

> The 'refresh' function  in 'builtin/add.c'
> declares 'flags' as signed, and passes it
> as an argument to the 'refresh_index' function,
> which though expects an unsigned value.

OK. This has become much easier to read by dropping the mention of the header file where the callee happens to be declared.

Just FYI, in my private rewrite (that I'll discard), I made it like so:

   builtin/add.c:refresh() declares 'flags' as signed, and uses it
   to call refresh_index() that expects an unsigned value.

> Since in this case 'flags' represents
> a bag of bits, whose MSB is not used
> in special ways, change the type
> of 'flags' to unsigned.

Good, too.

Will queue.

Show 18 quoted lines

> Signed-off-by: Eugenio Gigante <giganteeugenio2@gmail.com>
> ---
>  builtin/add.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index ada7719561..393c10cbcf 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -115,7 +115,7 @@ static int refresh(int verbose, const struct pathspec *pathspec)
>  	int i, ret = 0;
>  	char *skip_worktree_seen = NULL;
>  	struct string_list only_match_skip_worktree = STRING_LIST_INIT_NODUP;
> -	int flags = REFRESH_IGNORE_SKIP_WORKTREE |
> +	unsigned int flags = REFRESH_IGNORE_SKIP_WORKTREE |
>  		    (verbose ? REFRESH_IN_PORCELAIN : REFRESH_QUIET);
>  
>  	seen = xcalloc(pathspec->nr, 1);