Re: [PATCH 1/3] path.c: fix uninitialized memory access
From: Jeff King <hidden>
Date: 2017-10-03 23:30:47
Subsystem:
the rest · Maintainer:
Linus Torvalds
On Tue, Oct 03, 2017 at 03:45:01PM -0700, Jonathan Nieder wrote:
When I first read the above, I thought it was going to be about a
NUL-terminated string that was missing a NUL. But in fact, the issue
is that strlen(path) can be < 2.
In other words, an alternative fix would be
if (*path == '.' && path[1] == '/') {
...
}
which would not require passing in 'len' or switching to index-based
arithmetic. I think I prefer it. What do you think?Yes, I think that approach is much nicer. I think you could even use skip_prefix. Unfortunately you have to play a few games with const-ness, but I think the resulting signature for cleanup_path() is an improvement:
diff --git a/path.c b/path.c
index 00ec04e7a5..2e09a7bce0 100644
--- a/path.c
+++ b/path.c@@ -34,11 +34,10 @@ static struct strbuf *get_pathname(void) return sb; } -static char *cleanup_path(char *path) +static const char *cleanup_path(const char *path) { /* Clean it up */ - if (!memcmp(path, "./", 2)) { - path += 2; + if (skip_prefix(path, "./", &path)) { while (*path == '/') path++; }
@@ -47,7 +46,7 @@ static char *cleanup_path(char *path) static void strbuf_cleanup_path(struct strbuf *sb) { - char *path = cleanup_path(sb->buf); + const char *path = cleanup_path(sb->buf); if (path > sb->buf) strbuf_remove(sb, 0, path - sb->buf); }
@@ -64,7 +63,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...) strlcpy(buf, bad_path, n); return buf; } - return cleanup_path(buf); + return (char *)cleanup_path(buf); } static int dir_prefix(const char *buf, const char *dir)