Re: [PATCH v2 01/12] path.c: add git_common_path() and strbuf_git_common_path()

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 21, 2016, 00:28 UTC
Message-ID: <CACsJy8BYmXi_a-sJ2Z+J-rd+DTyQYnznc87_2EHfv_Ym-Fz0pg@mail.gmail.com>
In-Reply-To: <CAPig+cSRQBJM9xFnDszC84Z3JaF83sOg7=z-w6nRQdcu6nA3Bw@mail.gmail.com>

On Thu, Apr 21, 2016 at 1:11 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 15 quoted lines

> On Wed, Apr 20, 2016 at 9:24 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>> diff --git a/path.c b/path.c
>> @@ -503,6 +503,35 @@ void strbuf_git_path_submodule(struct strbuf *buf, const char *path,
>> +const char *git_common_path(const char *fmt, ...)
>> +{
>> +       struct strbuf *pathname = get_pathname();
>> +       va_list args;
>> +       va_start(args, fmt);
>> +       do_git_common_path(pathname, fmt, args);
>> +       va_end(args);
>> +       return pathname->buf;
>
> Is the caller expected to free this value? If not, then shouldn't
> 'pathname' be static? If so, then perhaps strbuf_detach() would be
> clearer (and return 'char *' rather than 'const char *').

get_pathname() actually holds a ring of static buffer. So no, we don't need static pathname, it can be a new buffer next time, mostly to to make printf("%s %s", git_common_path(..), git_common_path(..)) work. And no the caller is not supposed to free it, a little bit more convenient.

-- 
Duy