Masahiro Yamada <masahiroy@xxxxxxxxxx> 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);
> }
