From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Fri, 08 Jun 2012 12:33:04 GMT
Subject: Re: [PATCHv6 1/4] Read (but not write) from $XDG_CONFIG_HOME/git/config file
Message-ID: <CABPQNSbSVznFbnm=iy4HdZ4PDC70UfbrqvvSKx9moy+9AOk+Rw@mail.gmail.com>
In-Reply-To: <20120608142601.Horde.Tq7UO3wdC4BP0e9ZGAy1EwA@webmail.minatec.grenoble-inp.fr>

On Fri, Jun 8, 2012 at 2:26 PM,  <nguyenhu@minatec.inpg.fr> wrote:
>
> Junio C Hamano <gitster@pobox.com> a écrit :
>
>>> +char *mkpathdup(const char *fmt, ...)
>>> +{
>>> +       char path[PATH_MAX];
>>> +       va_list args;
>>> +       unsigned len;
>>> +
>>> +       va_start(args, fmt);
>>> +       len = vsnprintf(path, sizeof(path), fmt, args);
>>> +       va_end(args);
>>> +       if (len >= sizeof(path))
>>> +               return xstrdup(bad_path);
>>> +       return xstrdup(cleanup_path(path));
>>> +}
>>
>>
>> Hrmph. If a new helper is introduced anyway, wouldn't it be a better
>> idea to get rid of the hardcoded PATH_MAX limitation, perhaps using
>> strbuf_vaddf() or something in the implementation of this function?
>
>
> What about this ?
>
>
> char *mkpathdup(const char *fmt, ...)
> {
>        char *path;
>        struct strbuf sb = STRBUF_INIT;
>        va_list args;
>
>        va_start(args, fmt);
>        strbuf_vaddf(&sb, fmt, args);
>        va_end(args);
>        path = sb.buf;
>
>        strbuf_release(&sb);
>        return xstrdup(cleanup_path(path));
>
> }

No, strbuf_release(&sb) frees 'sb.buf', causing 'path' to point to
unallocated memory. You can fix that by doing something along these
lines on top:

 	va_end(args);
-	path = sb.buf;
+	path = xstrdup(cleanup_path(path));

 	strbuf_release(&sb);
-	return xstrdup(cleanup_path(path));
+	return path;
 }