git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

We 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

Previous: Jeff KingNext: Jacob Keller
Message 15 of 18 in “submodule inline diff format”
  1. 0/8 submodule inline diff formatJacob Keller, Aug 25, 2016
  2. 2/8 diff.c: remove output_prefix_length fieldJacob Keller, Aug 25, 2016
  3. 3/8 graph: add support for --line-prefix on all graph-aware outputJacob Keller, Aug 25, 2016
  4. 4/8 diff: prepare for additional submodule formatsJacob Keller, Aug 25, 2016
  5. 5/8 allow do_submodule_path to work even if submodule isn't checked outJacob Keller, Aug 25, 2016
  6. Junio C HamanoAug 26, 2016
  7. Keller, Jacob EAug 26, 2016
  8. 6/8 submodule: convert show_submodule_summary to use struct object_id *Jacob Keller, Aug 25, 2016
  9. 7/8 submodule: refactor show_submodule_summary with helper functionJacob Keller, Aug 25, 2016
  10. 8/8 diff: teach diff to display submodule difference with an inline diffJacob Keller, Aug 25, 2016
  11. 1/8 cache: add empty_tree_oid object and helper functionJacob Keller, Aug 25, 2016
  12. Stefan BellerAug 26, 2016
  13. Keller, Jacob EAug 26, 2016
  14. Jeff KingAug 26, 2016
  15. Jacob KellerAug 26, 2016
  16. Jacob KellerAug 26, 2016
  17. Junio C HamanoAug 31, 2016
  18. Jacob KellerAug 31, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.