Re: [PATCH 3/5] path.c: Use vsnpath() in the implementation of git_path()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 4, 2012, 20:30 UTC
- Message-ID
- <7vsjax8rap.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <50463A72.2000209@ramsay1.demon.co.uk>
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
Show 9 quoted lines
> The current implementation of git_path() is essentially the same as > that of vsnpath(), with two minor differences. First, git_path() > currently insists that the git directory path is no longer than > PATH_MAX-100 characters in length. However, vsnpath() does not > attempt this arbitrary 100 character reservation for the remaining > path components. Second, vsnpath() uses the "is_dir_sep()" macro, > rather than comparing directly to '/', to determine if the git_dir > path component ends with a path separator. > In order to benefit from the above improvements,...
In the longer term, I think this goes in the right direction, but the loss of reservation, especially when we know git_path() is used by some callers to get the base directory in $GIT_DIR that want to append stuff after the returned directory path to form the final pathname, is a bit worrysome. It may be hiding a bug (lack of proper limit check) on the callers' side.
Show 40 quoted lines
> ... along with increased
> compatability with git_snpath() and git_pathdup(), we reimplement the
> git_path() function using vsnpath().
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
> path.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/path.c b/path.c
> index 741ae77..cbbdf7d 100644
> --- a/path.c
> +++ b/path.c
> @@ -119,23 +119,14 @@ char *mkpath(const char *fmt, ...)
>
> char *git_path(const char *fmt, ...)
> {
> - const char *git_dir = get_git_dir();
> char *pathname = get_pathname();
> va_list args;
> - unsigned len;
> + char *ret;
>
> - len = strlen(git_dir);
> - if (len > PATH_MAX-100)
> - return bad_path;
> - memcpy(pathname, git_dir, len);
> - if (len && git_dir[len-1] != '/')
> - pathname[len++] = '/';
> va_start(args, fmt);
> - len += vsnprintf(pathname + len, PATH_MAX - len, fmt, args);
> + ret = vsnpath(pathname, PATH_MAX, fmt, args);
> va_end(args);
> - if (len >= PATH_MAX)
> - return bad_path;
> - return cleanup_path(pathname);
> + return ret;
> }
>
> void home_config_paths(char **global, char **xdg, char *file)