Re: [PATCH] merge-file: correctly find files when called in subdir

From: Thomas Rast <trast@student.ethz.ch>
Date: Oct 17, 2010, 10:39 UTC
Message-ID: <201010171239.13990.trast@student.ethz.ch>
In-Reply-To: <20101017011111.GA26656@burratino>

Jonathan Nieder wrote:

Show 27 quoted lines

> Bert Wesarg wrote:
> > On Sat, Oct 16, 2010 at 13:33, Thomas Rast <trast@student.ethz.ch> wrote:
> 
> >> --- a/builtin/merge-file.c
> >> +++ b/builtin/merge-file.c
> >> @@ -65,10 +66,18 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
> [...]
> >> +               if (prefix)
> >> +                       name = prefix_filename(prefix, prefixlen, argv[i]);
> >> +               else
> >> +                       name = argv[i];
> >
> > I think you can safe this condition, if you set prefixlen to 0.
> 
> I'm not so sure.  On Windows, prefix_filename() starts with
> 
> 	char *p;
> 	/* don't add prefix to absolute paths, but still replace '\' by '/' */
> 	if (is_absolute_path(arg))
> 		pfx_len = 0;
> 	else
> 		memcpy(path, pfx, pfx_len);
> 
> and memcpy() has undefined behavior when pfx is NULL (even with
> pfx_len of 0; see WG14/N1256: 7.21.1 "String function conventions").
> Even if the win32api memcpy does the right thing with NULL pfx, I
> cannot trust GCC when they are given that kind of license. :)

Hmmh.

I was ready to say "you're right" to Bert's suggestion. So maybe, for the sake of foolproof-ness, we should do something like the patch below?

diff --git i/setup.c w/setup.c
index a3b76de..7c8906d 100644
--- i/setup.c
+++ w/setup.c
@@ -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 && *pfx)
 		memcpy(path, pfx, pfx_len);
 	strcpy(path + pfx_len, arg);
 	for (p = path + pfx_len; *p; p++)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch