Re: [PATCH 2/2] Make a "gdb" dprintf that tells us load addresses.

[Glenn Washburn]

On Fri, 15 Oct 2021 16:55:17 -0400
Robbie Harwood <rharwood@redhat.com> wrote:

> Glenn Washburn <development@efficientek.com> writes:
> 
> > Robbie Harwood <rharwood@redhat.com> wrote:
> >> Glenn Washburn <development@efficientek.com> writes:
> >> 
> >>> I don't agree with suppressing file:line for debug output. I have
> >>> some changes in the pipeline which will allow selectively disabling
> >>> conditions when also using "debug=all". Suppressing file:line makes
> >>> it harder to figure out where the debug output is coming from to
> >>> find the condition to disable it.
> >> 
> >> Grep for add-symbol-file or /usr/lib/debug etc. will get there.
> >
> > I'm not following. Are you suggesting grepping for "add-symbol-file"
> > in the source? If so, I don't find that an acceptable answer to the
> > sysadmin trying to figure out hwo to disable that log message.
> 
> I don't understand your target audience - they have access to the
> source, but can't search it?  Do I have that right?

No, my target audience is a sysadmin trying to debug why grub is
failing in some way. They might not even be a programmer. But they can
read documentation and make intuitive guesses, but they likely don't
know how to use gdb. Say they are trying to debug an issue, but they
don't know exactly where things are going wrong, but it is reproducible.
This is on a system where all they have is a keyboard and a monitor.
So when the output scrolls off the top of the screen its gone.

They turn on debug=all because they don't know in what conditional(s)
would give them useful debug information. This will spew a lot of debug
messages to the screen, including the messages output by this patch.
The sysadmin may not see anything of value in the output, but will see
lots of messages of no apparent value. It would be nice to using this
output to be able to selectively disable it at runtime.

I forgot to mention that I have another patch that adds the conditional
used to the message to file:line:conditional is prepended. That
combined with a patch that allows selective disabling of debug log
message by conditional, allows the user to iteratively pare down debug
log messages so that the ones of value have a greater likelihood of
being on the limited screen realestate. This would look like, for
example:

  set debug=all,-gdb,-disk,-lexer,-alloc

Now the patch as is does not show the file:line prefix, and wouldn't
show the conditional after my patch. So the user has no way of knowing
what conditional should be used to disable the gdb output. And may
actually be confused because all debug messages are currently prefixed
the file:line, even multiline messages. So the user may thing that the
gdb message is a multiline message that comes from the debug log
message that preceeded it, leading them to disable potentially the
wrong conditional.

So, no, I think it should remain the standard that log messages enabled
via the debug variable always have a file:line prefix.

On the otherhand, since you're able to copy paste, I'm guessing you're
on a serial line or VM and not on in the situation I'm describing. In
which case, without the proposed "\n" modification, having the file:line
prefix is an extremely minor inconvenience (you'll have to find the
start of the add-symbol-file, which won't be at the beginning of the
line). And you can copy-paste, you likely can search that output
anyway. The person in the scenario I describe doesn't have that luxury.

Why do I care? I've been that guy trying to figure out a boot issue
with too much noise in the debug log messages pushing the messages I
needed to see off the screen (and the source wasn't readily available).
And no pager is not always an option if you have hundreds of pages of
log messages to scroll through, it takes forever.

> If so, it seems unlikely that we're going to agree.

I don't think we need to agree (that is for you to agree that the issue
I'm describing is something to be concerned about). However, like I
said, I think we can both be satisfied by not leaving out the file:line
and to prepend "\n" to "add-symbol-file" so that it will start on a new
line. So I'll ask again. Is my proposal an acceptable modification for
the need that you have?

> Be well,
> --Robbie

Cheers,
Glenn