Re: [GSoC PATCH v2] pathspec: fix sign comparison warnings

[Junio C Hamano <gitster@pobox.com>]

Arnav Bhate <bhatearnav@gmail.com> writes:

> @@ -43,12 +42,12 @@ void add_pathspec_matches_against_index(const struct pathspec *pathspec,
>  	 * mistakenly think that the user gave a pathspec that did not match
>  	 * anything.
>  	 */
> -	for (i = 0; i < pathspec->nr; i++)
> +	for (int i = 0; i < pathspec->nr; i++)
>  		if (!seen[i])
>  			num_unmatched++;
>  	if (!num_unmatched)
>  		return;
> -	for (i = 0; i < istate->cache_nr; i++) {
> +	for (unsigned int i = 0; i < istate->cache_nr; i++) {
>  		const struct cache_entry *ce = istate->cache[i];
>  		if (sw_action == PS_IGNORE_SKIP_WORKTREE &&
>  		    (ce_skip_worktree(ce) || !path_in_sparse_checkout(ce->name, istate)))

Makes me wonder if it is a nicer solution for longer term to count
items in "struct pathspec" with unsigned, not signed int.  A local
variable that needs to hold the number of items plus an extra bit
that signals an invalid state (typically denoted by a negative
number) needs to be signed, but a member in a struct that records
number of items in an array in the same struct has no reason to be
of signed type, as the invalid state could just be represented by
the .item being NULL.

But let's leave it for later; it is not something we want to leave
GSoC students to handle.

> @@ -845,7 +844,7 @@ int pathspec_needs_expanded_index(struct index_state *istate,
>  			 * - not-in-cone/bar*: may need expanded index
>  			 * - **.c: may need expanded index
>  			 */
> -			if (strspn(item.original + item.nowildcard_len, "*") == item.len - item.nowildcard_len &&
> +			if (strspn(item.original + item.nowildcard_len, "*") == (unsigned int)(item.len - item.nowildcard_len) &&
>  			    path_in_cone_mode_sparse_checkout(item.original, istate))
>  				continue;
>  
> @@ -860,7 +859,7 @@ int pathspec_needs_expanded_index(struct index_state *istate,
>  				 * directory name and the sparse directory is the first
>  				 * component of the pathspec, need to expand the index.
>  				 */
> -				if (item.nowildcard_len > ce_namelen(ce) &&
> +				if ((unsigned int)item.nowildcard_len > ce_namelen(ce) &&
>  				    !strncmp(item.original, ce->name, ce_namelen(ce))) {
>  					res = 1;
>  					break;

These lines in these two hunks are way overly long already in the
original, and extra casts make the even worse.  Perhaps fold them
in the middle appropriately?