[PATCH v2 1/2] prefix_filename(): safely handle the case where pfx_len=0
- From
Thomas Rast <trast@student.ethz.ch>
- Date
- Oct 17, 2010, 19:23 UTC
- Message-ID
- <64b470380b8f7bd8afe8f019553cb99f72091c7d.1287342969.git.trast@student.ethz.ch>
- In-Reply-To
- <20101017160546.GD301@burratino>
Current prefix_filename() is proofed against the case where the prefix 'pfx' is NULL or a 0-length string, _except on Windows_.
Change the behaviour to work the same on both platforms, and only check pfx_len so that callers passing a NULL prefix with a nonzero pfx_len segfault early on both.
Signed-off-by: Thomas Rast <trast@student.ethz.ch> ---
Jonathan Nieder wrote:
Show 8 quoted lines
> Yes, please! But I'd write it as > > if (pfx_len) > memcpy(...) > > and rely on the commit message to explain that this is about the !pfx > case (so callers with pfx_len == 3, !pfx get a nice, quick segfault > instead of heisenbugs from uninitialized data).
Hrm, why not. I think symmetry is important here, so I also changed the non-Windows arm, which feels wrong somehow. But it didn't break any tests.
In writing this I also realised that "it didn't break any tests" is pretty weak and checked them all by hand (there are only a dozen grep matches).
setup.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c index a3b76de..346ef2e 100644 --- a/setup.c +++ b/setup.c @@ -46,7 +46,7 @@ { static char path[PATH_MAX]; #ifndef WIN32 - if (!pfx || !*pfx || is_absolute_path(arg)) + if (!pfx_len || is_absolute_path(arg)) return arg; memcpy(path, pfx, pfx_len); strcpy(path + pfx_len, arg); @@ -55,7 +55,7 @@ /* don't add prefix to absolute paths, but still replace '\' by '/' */ if (is_absolute_path(arg)) pfx_len = 0; - else + else if (pfx_len) memcpy(path, pfx, pfx_len); strcpy(path + pfx_len, arg); for (p = path + pfx_len; *p; p++)
-- 1.7.3.1.266.g3c065