Re: [PATCH] gdb: add linux_nat_debug_printf macro

Joel Brobecker
2020-07-13T14:32:45Z

Hi Simon,

On Fri, Jul 10, 2020 at 09:55:40AM -0400, Simon Marchi wrote:
> On 2020-07-10 7:47 a.m., Gary Benson via Gdb-patches wrote:
> > Simon Marchi wrote:
> >> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> >> index fde360f5080a..3b5c803b83fd 100644
> >> --- a/gdb/linux-nat.c
> >> +++ b/gdb/linux-nat.c
> >> @@ -198,6 +198,27 @@ show_debug_linux_nat (struct ui_file *file, int from_tty,
> >> value);
> >> }
> >>
> >> +/* Print a debug statement. Should be used through linux_nat_debug_printf. */
> >> +
> >> +static void ATTRIBUTE_PRINTF (2, 3)
> >> +linux_nat_debug_printf_1 (const char *func_name, const char *fmt, ...)
> >> +{
> >> + fprintf_unfiltered (gdb_stdlog, "[linux-nat] %s: ", func_name);
> >> +
> >> + va_list ap;
> >> + va_start (ap, fmt);
> >> + vfprintf_unfiltered (gdb_stdlog, fmt, ap);
> >> + va_end (ap);
> >> +
> >> + fprintf_unfiltered (gdb_stdlog, "\n");
> >> +}
> >
> > Would it be useful to use debug_printf and debug_vprintf here?
> >
> > Thanks,
> > Gary
>
> Indeed, I think the result would be the same, and it seems to exist for this purpose. I've
> replaced locally the implementation of linux_nat_debug_printf_1 with:
>
> static void ATTRIBUTE_PRINTF (2, 3)
> linux_nat_debug_printf_1 (const char *func_name, const char *fmt, ...)
> {
> debug_printf ("[linux-nat] %s: ", func_name);
>
> va_list ap;
> va_start (ap, fmt);
> debug_vprintf (fmt, ap);
> va_end (ap);
>
> debug_printf ("\n");
> }

I skimmed through the patch, and it looked good to me.

Thanks again for doing that. That's a nice improvement.

Thinking out loud (and certainly not a suggestion or request that
you take care of the implementation): I'm wondering if we should be
thinking about generalizing this approach, somehow. For instance,
if we a function that took not just a func name, but also the name
of the unit:

unit_debug_vprintf (const char *module_name, const char *func_name,
const char *fmt, ...)

... or perhaps more generally:

unit_debug_vprintf (const char *label, const char *func_name,
const char *fmt, ...)

... then all modules would have to do to have consistent logging
is to define one macro.

--
Joel