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

Re: [PATCH 4/8] worktree: drop useless call to strbuf_reset

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Sep 10, 2020, 19:49 UTC
Message-ID
<CAPig+cSEOUebNV90Gec2KqskZgAmX=jd++s4tfAyYN6Up4_O3A@mail.gmail.com>
In-Reply-To
<CAN0heSrQT9N3=e70qkgS_rOQ0oy0rrHqud=rRtr-r5JaL=ofNQ@mail.gmail.com>
On Thu, Sep 10, 2020 at 3:40 PM Martin Ågren <martin.agren@gmail.com> wrote:
Show 12 quoted lines
> On Thu, 10 Sep 2020 at 21:15, Eric Sunshine <sunshine@sunshineco.com> wrote:
> > I think this patch is wrong and should be dropped. That strbuf is
> > static, and the called strbuf_worktree_ref() does not reset it, so
> > each call to worktree_ref() will now merely append to the existing
> > content (which is undesirable) following this change.
>
> That's not to say this optimization won't ever be useful, of course. I
> also begin to hope that no caller keeps their returned pointer around
> for long. It only seems to be used from `other_ref_heads()` and that
> looks ok. If we do want this strbuf reuse, maybe that function could
> just keep its own strbuf and reuse it (not necessarily having it be
> static) and learn not to call `worktree_ref(wt, "HEAD")` twice.

Yep, I wouldn't be unhappy to see worktree_ref() disappear altogether. There are no external callers and it would be easy enough to retrofit the lone internal caller to use the safer strbuf_worktree_ref() anyhow. Plus, both calls to worktree_ref() in other_head_refs() invoke it with the exact same arguments, `worktree_ref(wt, "HEAD")`, which makes one wonder if it need be called twice at all in that particular scenario.

Previous: Martin ÅgrenNext: Martin Ågren
Message 18 of 33 in “various wt-status/worktree cleanups”
  1. 0/8 various wt-status/worktree cleanupsMartin Ågren, Sep 10, 2020
  2. 6/8 worktree: rename copy-pasted variableMartin Ågren, Sep 10, 2020
  3. Junio C HamanoSep 10, 2020
  4. Martin ÅgrenSep 12, 2020
  5. Martin ÅgrenSep 27, 2020
  6. 7/8 worktree: use skip_prefix to parse targetMartin Ågren, Sep 10, 2020
  7. 1/8 wt-status: replace sha1 mentions with oidMartin Ågren, Sep 10, 2020
  8. 2/8 wt-status: print to s->fp, not stdoutMartin Ågren, Sep 10, 2020
  9. 3/8 wt-status: introduce wt_status_state_free_buffers()Martin Ågren, Sep 10, 2020
  10. 8/8 worktree: simplify search for unique worktreeMartin Ågren, Sep 10, 2020
  11. Eric SunshineSep 10, 2020
  12. Martin ÅgrenSep 10, 2020
  13. Eric SunshineSep 10, 2020
  14. Junio C HamanoSep 10, 2020
  15. 4/8 worktree: drop useless call to strbuf_resetMartin Ågren, Sep 10, 2020
  16. Eric SunshineSep 10, 2020
  17. Martin ÅgrenSep 10, 2020
  18. Eric SunshineSep 10, 2020
  19. Martin ÅgrenSep 12, 2020
  20. 5/8 worktree: update renamed variable in commentMartin Ågren, Sep 10, 2020
  21. Taylor BlauSep 12, 2020
  22. Martin ÅgrenSep 12, 2020
  23. 0/7 various wt-status/worktree cleanupsMartin Ågren, Sep 27, 2020
  24. 1/7 wt-status: replace sha1 mentions with oidMartin Ågren, Sep 27, 2020
  25. 2/7 wt-status: print to s->fp, not stdoutMartin Ågren, Sep 27, 2020
  26. 4/7 worktree: inline `worktree_ref()` into its only callerMartin Ågren, Sep 27, 2020
  27. Eric SunshineSep 28, 2020
  28. Martin ÅgrenSep 28, 2020
  29. Eric SunshineSep 28, 2020
  30. 3/7 wt-status: introduce wt_status_state_free_buffers()Martin Ågren, Sep 27, 2020
  31. 5/7 worktree: update renamed variable in commentMartin Ågren, Sep 27, 2020
  32. 6/7 worktree: rename copy-pasted variableMartin Ågren, Sep 27, 2020
  33. 7/7 worktree: use skip_prefix to parse targetMartin Ågren, Sep 27, 2020

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.