Re: [PATCH v2 0/4] road to reentrant real_path

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 9, 2016, 12:33 UTC
Message-ID: <CACsJy8A2M_G34MeHh6vGsrf5ePOOduM6u=n17_EZLtu31uDAYg@mail.gmail.com>
In-Reply-To: <1481241494-6861-1-git-send-email-bmwill@google.com>

On Fri, Dec 9, 2016 at 6:58 AM, Brandon Williams <bmwill@google.com> wrote:

Show 10 quoted lines

> diff --git a/setup.c b/setup.c
> index fe572b8..0d9fdd0 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -254,10 +254,12 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
>                 if (!is_absolute_path(data.buf))
>                         strbuf_addf(&path, "%s/", gitdir);
>                 strbuf_addbuf(&path, &data);
> -               strbuf_addstr(sb, real_path(path.buf));
> +               strbuf_realpath(sb, path.buf, 1);

This is not the same because of this hunk in strbuf_realpath()

Show 8 quoted lines

> @@ -81,17 +73,18 @@ static const char *real_path_internal(const char *path, int die_on_error)
>                         goto error_out;
>         }
>
> -       strbuf_reset(&resolved);
> +       strbuf_reset(resolved);
>
>         if (is_absolute_path(path)) {

But if you you remove that, then all (old) callers of strbuf_realpath() must do a strbuf_reset() in advance if needed (probably just real_path does) which sounds reasonable to me. You're probably want to be careful about the strbuf_reset() at the end of the function too.

Other than that, I think this diff looks nice.

-- 
Duy