On Fri, Nov 06, 2015 at 11:47:03PM +0100, René Scharfe wrote:
> When a branch name is longer than four characters, memcmp() can read
> past the end of the string literal "HEAD". Use strncmp() instead, which
> stops at the end of a string. This fixes the following test failures
> with AddressSanitizer:
Hmm. I think this is mostly harmless, as a comparison like:
memcmp("HEAD and more", "HEAD", strlen("HEAD"))
would yield non-zero when we compare the NUL in the second string to
whatever is in the first. So I assume what is going on is that memcmp is
doing larger compares than byte by byte, and is examining 4 or 8 bytes
starting at that NUL.
The outcome is equivalent, but we do touch memory that is not ours, so I
think this is a positive direction in that sense.
But...
> diff --git a/wt-status.c b/wt-status.c
> index 435fc28..8dc281b 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1319,7 +1319,7 @@ static int grab_1st_switch(unsigned char *osha1,
> unsigned char *nsha1,
> hashcpy(cb->nsha1, nsha1);
> for (end = target; *end && *end != '\n'; end++)
> ;
> - if (!memcmp(target, "HEAD", end - target)) {
> + if (!strncmp(target, "HEAD", end - target)) {
This will match prefixes like "HEA" in the target, won't it?
I think you want something more like:
if (end - target == 4 && !memcmp(target, "HEAD", 4))
I tried to think of a way that didn't involve a magic number. The best I
came up with is:
if (skip_prefix(target, "HEAD", &v) && v == end)
but that requires an extra variable, and is arguably more obfuscated.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
