Re: [PATCH] gdb: convert auto-load to new-style debug macros

Simon Marchi via Gdb-patches
2021-01-21T19:11:27Z

On 2021-01-21 2:05 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> Function file_is_auto_load_safe was taking a format string and varargs
> Simon> just to output a debug print. This is probably because that function is
> Simon> used in linux-thread-db.c and main.c, but debug_auto_load is static in
> Simon> auto-load.c. I simplified that, making debug_auto_load visible outside
> Simon> of auto-load.c, and making the callers of file_is_auto_load_safe output
> Simon> the debug print themselves.
>
> Simon> The rest is pretty much standard.
>
> Simon> gdb/ChangeLog:
>
> Simon> * auto-load.h (debug_auto_load): Move here.
> Simon> (auto_load_debug_printf): New.
> Simon> * auto-load.c: Use auto_load_debug_printf.
> Simon> (debug_auto_load): Move to header.
> Simon> * linux-thread-db.c (try_thread_db_load): Use
> Simon> auto_load_debug_printf.
> Simon> * main.c (captured_main_1): Likewise.
>
> Thanks. This seems like a nice cleanup.
> I have one comment.
>
> Simon> - _("auto-load: Loading .gdbinit "
> Simon> - "file \"%s\".\n"),
> Simon> - local_gdbinit.c_str ()))
> Simon> + if (!inhibit_gdbinit && auto_load_local_gdbinit)
> Simon> {
> Simon> - auto_load_local_gdbinit_loaded = 1;
> Simon> + auto_load_debug_printf ("Loading .gdbinit file \"%s\".",
> Simon> + local_gdbinit.c_str ());
>
> Some of these changes lose the _() wrapper.
> I don't know if we care for debug logging .. or maybe we actively want
> to remove them in this case? Anyway I thought I'd point it out.

I removed them on purpose (will add a mention in the commit log), because
I have not seen them used for logging elsewhere. I don't really think
it's necessary.

I'll push with the mention added, and if I stumble on other places that
uses _ for logging, I'll remove it.

Thanks,

Simon