On Thu, Feb 16, 2023 at 10:34:40PM +0000, Matthew John Cheetham via GitGitGadget wrote:
> +/*
> + * Like skip_prefix_mem, but compare case-insensitively. Note that the
> + * comparison is done via tolower(), so it is strictly ASCII (no multi-byte
> + * characters or locale-specific conversions).
> + */
> +static inline int skip_iprefix_mem(const char *buf, size_t len,
> + const char *prefix,
> + const char **out, size_t *outlen)
> +{
> + size_t prefix_len = strlen(prefix);
> +
> + if (len < prefix_len)
> + return 0;
> +
> + if (!strncasecmp(buf, prefix, prefix_len)) {
> + *out = buf + prefix_len;
> + *outlen = len - prefix_len;
> + return 1;
> + }
> +
> + return 0;
> +}
The comment at the top of the function seems out of date. It's using
strncasecmp(), so it probably would be locale-dependent. I think that's
probably OK, but we should probably fix the comment.
Alternatively, you could copy the tolower() loop from skip_iprefix().
Something like:
diff --git a/git-compat-util.h b/git-compat-util.h
index 28456241b6..f671a0ec3f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1296,17 +1296,13 @@ static inline int skip_iprefix_mem(const char *buf, size_t len,
const char *prefix,
const char **out, size_t *outlen)
{
- size_t prefix_len = strlen(prefix);
-
- if (len < prefix_len)
- return 0;
-
- if (!strncasecmp(buf, prefix, prefix_len)) {
- *out = buf + prefix_len;
- *outlen = len - prefix_len;
- return 1;
- }
-
+ do {
+ if (!*prefix) {
+ *out = buf;
+ *outlen = len;
+ return 1;
+ }
+ } while (len-- > 0 && tolower(*buf++) == tolower(*prefix++));
return 0;
}
looks right to me, though only lightly tested (via t5563). I'm happy
with either implementation.
> +static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p)
> [...]
> + /*
> + * If this is a HTTP status line and not a header field, this signals
> + * a different HTTP response. libcurl writes all the output of all
> + * response headers of all responses, including redirects.
> + * We only care about the last HTTP request response's headers so clear
> + * the existing array.
> + */
> + if (!strncasecmp(ptr, "http/", 5))
> + strvec_clear(values);
Since "ptr" isn't NUL terminated, using strncasecmp() may walk off the
end. I think you'd need to check that there are five bytes. You could
even use skip_iprefix_mem(), though of course we'd throw away the output
values. (For strings there is also istarts_with(), but I don't think we
have a "mem" equivalent).
The rest of the parsing looks good to me.
-Peff
