Re: [PATCH 1/3] make_absolute_path: Don't try to copy a string to itself

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2011, 20:25 UTC
Message-ID: <7v39mpcuv9.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1300130318-11279-2-git-send-email-cmn@elego.de>

Carlos Martín Nieto <cmn@elego.de> writes:

> Sometimes (at least in t-0001-init.sh test 12), the return value of
> make_absolute_path() is passed to it as an argument, making the first

I don't think it is a bad idea per-se to avoid a copy from the same memory location into the same memory location, but independent of the necessity of fixes at the low-level, shouldn't we fix the callers that do not check if what they have is already absolute?

Show 8 quoted lines

> and second arguments to strlcpy() the same, making the test fail when
> run under valgrind.
>
> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> ---
>
> This patch assumes the path returned by make_absolute_path() is never
> longer than PATH_MAX, which I think is a safe assumption.

Show 17 quoted lines

>
>  abspath.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/abspath.c b/abspath.c
> index 91ca00f..9149a98 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -24,7 +24,7 @@ const char *make_absolute_path(const char *path)
>  	char *last_elem = NULL;
>  	struct stat st;
>  
> -	if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
> +	if (buf != path && strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
>  		die ("Too long path: %.*s", 60, path);
>  
>  	while (depth--) {