Re: [PATCH v11 0/8] submodule inline diff format
- From
Jacob Keller <jacob.keller@gmail.com>
- Date
- Aug 26, 2016, 23:05 UTC
- Message-ID
- <CA+P7+xpvtdoeK3uiSGAeNzQaYQhS7p0WT+nCcfKAS7FauFNftQ@mail.gmail.com>
- In-Reply-To
- <20160826200426.l3eu2mojgmjdz2ec@sigill.intra.peff.net>
On Fri, Aug 26, 2016 at 1:04 PM, Jeff King <peff@peff.net> wrote:
Show 28 quoted lines
> On Fri, Aug 26, 2016 at 07:58:07PM +0000, Keller, Jacob E wrote:
>
>> > > char *git_pathdup_submodule(const char *path, const char *fmt,
>> > > ...)
>> > > {
>> > > + int err;
>> > > va_list args;
>> > > struct strbuf buf = STRBUF_INIT;
>> > > va_start(args, fmt);
>> > > - do_submodule_path(&buf, path, fmt, args);
>> > > + err = do_submodule_path(&buf, path, fmt, args);
>> > > va_end(args);
>> > > + if (err)
>> >
>> > Here we need a strbuf_release(&buf) to avoid a memory leak?
>>
>> No, cause we "strbuf_detach" after this to return the buffer? Or is
>> that not safe?
>
> That code path is OK. I think the question is whether you need to
> release the buffer in the "err" case where you return NULL and don't hit
> the strbuf_detach.
>
> IOW, does do_submodule_path() promise that when it returns an error,
> "buf" has been left uninitialized? Some of our strbuf functions do, but
> I do not know offhand about do_submodule_path().
>
> -PeffWe probably should release for the error case. I'll do that. I don't believe do_submodule_path ensures that the passed in argument is guaranteed to not be initialized or used.
Thanks, Jake