Re: [PATCH v11 0/8] submodule inline diff format

From: Jeff King <peff@peff.net>
Date: Aug 26, 2016, 20:04 UTC
Message-ID: <20160826200426.l3eu2mojgmjdz2ec@sigill.intra.peff.net>
In-Reply-To: <1472241486.28343.10.camel@intel.com>

On Fri, Aug 26, 2016 at 07:58:07PM +0000, Keller, Jacob E wrote:

Show 16 quoted lines

> > >  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().

-Peff