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

From
Junio C Hamano <hidden>
Date
2023-02-27 20:07:30
Masahiro Yamada [off-list ref] 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 ...
quoted hunk
 		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?
quoted hunk
-						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?
quoted hunk
 							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);
 }
Thread (1 message) 1 message, 1 author, last reply 2023-02-27
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help