Re: [PATCH] git-mv: Fix error with multiple sources.

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2010, 06:24 UTC
Message-ID: <7vd412ac8c.fsf@alter.siamese.dyndns.org>
In-Reply-To: <718290.769818367-sendEmail@darysmbp>

"David Rydh" <dary@math.berkeley.edu> writes:

Show 13 quoted lines

> diff --git a/builtin-mv.c b/builtin-mv.c
> index 8247186..1c1f8be 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -27,7 +27,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
>  		if (length > 0 && is_dir_sep(result[i][length - 1]))
>  			result[i] = xmemdupz(result[i], length - 1);
>  		if (base_name)
> -			result[i] = basename((char *)result[i]);
> +			result[i] = xstrdup(basename((char *)result[i]));
>  	}
>  	return get_pathspec(prefix, result);
>  }

Given that basename(3) is allowed to modify its parameter, I think the above code is still not portable. casting constness away and feeding result[i], especially when we didn't obtain our own copy by calling xmemdupz(), is especially problematic.

Perhaps something ugly like this?

	for (i = 0; i < count; i++) {
		int length = strlen(result[i]);
		int to_copy = length;
                while (to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
			to_copy--;
		if (to_copy != length || basename) {
                	char *it = xmemdupz(result[i], to_copy);
                        result[i] = base_name ? strdup(basename(it)) : it;
		}
	}