Re: [PATCH v2 4/5] wildmatch: use char instead of uchar

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

Masahiro Yamada <masahiroy@kernel.org> writes:

> GIT has its own implementations, so the behavior is clear.
>
> In fact, commit 4546738b58a0 ("Unlocalized isspace and friends")
> says one of the motivations is "we want the right signed behaviour".
>
> sane_istest() casts the given character to (unsigned char) anyway
> before sane_ctype[] table lookup, so dowild() can use 'char'.

Use of values taken from a char/uchar in sane_istest() is designed
to be safe, and between using char*/uchar* to scan pieces of memory
would not make much difference, so changes like ...

>  		case '*':
>  			if (*++p == '*') {
> -				const uchar *prev_p = p - 2;
> +				const char *prev_p = p - 2;

... this is safe, and so is ...

> -				const char *slash = strchr((char*)text, '/');
> +				const char *slash = strchr(text, '/');

... this.

But does the comparison between t_ch_upper and prev_ch behave the
same with and without this patch?

> -						uchar t_ch_upper = toupper(t_ch);
> +						char t_ch_upper = toupper(t_ch);
>  						if (t_ch_upper <= p_ch && t_ch_upper >= prev_ch)

Here t_ch is already known to pass islower(), so t_ch_upper would be
within a reasonable range and "char" should be able to store it
safely without its value wrapping around to negative.  Do we have a
similar guarantee on prev_ch, or can it be more than 128, which
would have caused the original to say "t_ch_upper is smaller than
prev_ch" but now because prev_ch can wrap around to a negative
value, leading the conditional to behave differently, or something?

>  							matched = 1;
>  					}
>  					p_ch = 0; /* This makes "prev_ch" get set to 0. */
>  				} else if (p_ch == '[' && p[1] == ':') {
> -					const uchar *s;
> +					const char *s;
>  					int i;
>  					for (s = p += 2; (p_ch = *p) && p_ch != ']'; p++) {} /*SHARED ITERATOR*/
>  					if (!p_ch)
> @@ -237,5 +235,5 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)
>  /* Match the "pattern" against the "text" string. */
>  int wildmatch(const char *pattern, const char *text, unsigned int flags)
>  {
> -	return dowild((const uchar*)pattern, (const uchar*)text, flags);
> +	return dowild(pattern, text, flags);
>  }