On Thu, Sep 06, 2018 at 09:52:28AM -0700, Junio C Hamano wrote: > Stefan Beller <sbeller@xxxxxxxxxx> 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?) -Peff