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

Re: [PATCH 01/11] string_list: print_string_list to use trace_printf

From
Stefan Beller <sbeller@google.com>
Date
Sep 6, 2018, 22:16 UTC
Message-ID
<CAGZ79kbRyVZ_1QpX0pPigN+2Pt2dV1eYvLyoWeSWejbLxko4YQ@mail.gmail.com>
In-Reply-To
<20180906165602.GA19367@sigill.intra.peff.net>
On Thu, Sep 6, 2018 at 9:56 AM Jeff King <peff@peff.net> wrote:
Show 36 quoted lines
>
> On Thu, Sep 06, 2018 at 09:52:28AM -0700, Junio C Hamano wrote:
>
> > Stefan Beller <sbeller@google.com> writes:
> >
> > > It is a debugging aid, so it should print to the debugging channel.
> >
> > ... and rename it with trace_ prefix.
> >
> > Use of trace_printf() is nice, as we can control its behavior at
> > runtime ;-)
>
> Yes, though...
>
> > > -void print_string_list(const struct string_list *p, const char *text)
> > > +void trace_print_string_list(const struct string_list *p, const char *text)
> > >  {
> > >     int i;
> > >     if ( text )
> > > -           printf("%s\n", text);
> > > +           trace_printf("%s\n", text);
> > >     for (i = 0; i < p->nr; i++)
> > > -           printf("%s:%p\n", p->items[i].string, p->items[i].util);
> > > +           trace_printf("%s:%p\n", p->items[i].string, p->items[i].util);
> > >  }
>
> It seems funny that we'd iterate through the list checking over and over
> whether tracing is enabled.
>
> Should this do:
>
>   if (!trace_want(&trace_default_key))
>         return;
>
> at the top? (Or possibly even take a trace key from the caller, so that
> it can use whatever context makes sense for this particular list?)

I added this check as well as rewording the commit message to recite Junios understanding of the patch as well.

However I would want to not derail this patch any further. This function was used as an aid by me some time ago, so I am willing to share the modifications needed for efficient printf debugging here, but I do not want to be dragged into a rabbit hole.

For example taking the trace key is much overkill IMHO for a pure debugging aid (and so is the shortcutting return that you proposed, but I added that already for the resend), so if anyone needs this function outside of printf-debugging, I would recommend patches on top.

Thanks, Stefan

Previous: Jeff KingNext: Jeff King
Message 5 of 39 in “fetch: make sure submodule oids are fetched”
  1. 00/11 fetch: make sure submodule oids are fetchedStefan Beller, Sep 4, 2018
  2. 01/11 string_list: print_string_list to use trace_printfStefan Beller, Sep 4, 2018
  3. Junio C HamanoSep 6, 2018
  4. Jeff KingSep 6, 2018
  5. Stefan BellerSep 6, 2018
  6. Jeff KingSep 7, 2018
  7. Junio C HamanoSep 7, 2018
  8. Stefan BellerSep 7, 2018
  9. 1/2 trace: add trace_print_string_list_keyStefan Beller, Sep 10, 2018
  10. 2/2 string-list: remove unused function print_string_listStefan Beller, Sep 10, 2018
  11. Junio C HamanoSep 10, 2018
  12. Stefan BellerSep 10, 2018
  13. Junio C HamanoSep 11, 2018
  14. Stefan BellerSep 11, 2018
  15. Jeff KingSep 11, 2018
  16. string-list: remove unused function print_string_listStefan Beller, Sep 11, 2018
  17. Junio C HamanoSep 11, 2018
  18. Junio C HamanoSep 11, 2018
  19. Stefan BellerSep 11, 2018
  20. Junio C HamanoSep 11, 2018
  21. 02/11 string-list.h: add string_list_{pop, last} functionsStefan Beller, Sep 4, 2018
  22. Junio C HamanoSep 6, 2018
  23. Stefan BellerSep 6, 2018
  24. 03/11 sha1-array: provide oid_array_filterStefan Beller, Sep 4, 2018
  25. Junio C HamanoSep 6, 2018
  26. 04/11 submodule.c: convert submodule_move_head new argument to object idStefan Beller, Sep 4, 2018
  27. Junio C HamanoSep 6, 2018
  28. 05/11 submodule.c: fix indentationStefan Beller, Sep 4, 2018
  29. Junio C HamanoSep 6, 2018
  30. 06/11 submodule.c: sort changed_submodule_names before searching itStefan Beller, Sep 4, 2018
  31. Junio C HamanoSep 6, 2018
  32. Stefan BellerSep 11, 2018
  33. 07/11 submodule: move global changed_submodule_names into fetch submodule structStefan Beller, Sep 4, 2018
  34. Junio C HamanoSep 6, 2018
  35. 08/11 submodule.c: do not copy around submodule listStefan Beller, Sep 4, 2018
  36. Junio C HamanoSep 6, 2018
  37. 09/11 submodule: fetch in submodules git directory instead of in worktreeStefan Beller, Sep 4, 2018
  38. 10/11 fetch: retry fetching submodules if sha1 were not fetchedStefan Beller, Sep 4, 2018
  39. 11/11 builtin/fetch: check for submodule updates for non branch fetchesStefan Beller, Sep 4, 2018

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.