Re: [PATCH 0/2] info breakpoints improvements

[Pedro Alves <pedro@palves.net>]

On 2022-05-24 14:14, Eli Zaretskii wrote:
>> Date: Mon, 23 May 2022 18:06:22 +0100
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>>
>>>>  (top-gdb) info breakpoints
>>>>  Num     Type           Disp Enb Address            What
>>>>  1       breakpoint     keep y                      internal_error
>>>>   1.1                        y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
>>>>  2       breakpoint     keep y                      -qualified internal_error
>>>>   2.1                        y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
>>>>  3       breakpoint     keep y                      errors.c:54
>>>>   3.1                        y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
>>>>   3.2                        y   0x00007ffff6d50410 in PyErr_SetObject at /usr/src/debug/python2-2.7.15-4.fc27.x86_64/Python/errors.c:54
>>>>  4       breakpoint     keep y                      gdb.c:27
>>>>   4.1                        y   0x000055555564107b in main(int, char**) at src/gdb/gdb.c:28
>>>>  (top-gdb)
>>>
>>> I must confess that the new display is much more cluttered, and
>>> includes redundant information, so it's harder to read.  It also makes
>>> the important stuff harder to find.  Why exactly is this deemed as
>>> improvement, and in particular, why would we want this behavior as the
>>> default?  (I won't mind to have this as opt-in behavior, if someone
>>> finds this useful in some situations.)
>>
>> None of the information is actually redundant.  The "Enb" column shows
>> a different "y", as you can disable a whole breakpoint, or its locations
>> independently.
> 
> In the "usual" case, such as the one below:
> 
>>>>  4       breakpoint     keep y                      gdb.c:127
>>>>   4.1                        y   0x000055555564107b in main(int, char**) at src/gdb/gdb.c:127
> 
> one of the two "y"s is redundant, as well as one of the two
> "gdb.c:127"s.  And in this case:
> 
>>>  1       breakpoint     keep y                      internal_error
>>>   1.1                        y   0x00000000005755a5 in internal_error(char const*, int, char const*, ...) at src/gdb/common/errors.c:54
> 
> the "internal_error" part of the "header row" is also redundant.
> 

Well, with that view, this is very hard to discuss.  The example quoted 
at the top shows that breakpoints 1 through 3 all have the same breakpoint
location in internal_error at src/gdb/common/errors.c:54 , yet, they were all
specified differently.  There is no way to distinguish them in the CLI today.

"internal_error" alone is not a source location, it's just a way to tell
GDB to find the locations where that function exists.  It's just a function name.
The source location is the address, filename and line number tuple.

>> The "What" column is showing different things for the breakpoint
>> and its locations.
> 
> Different, but in many cases quite similar, with many common parts.

Sure.  But not always, and thus I find it important to show the missing
info, so we can tell which is which.

>> So, a more concise display may be interesting, but I'd like to think of it as
>> a separate feature.
> 
> It's okay to disagree about user perspectives on what a "concise"
> display should and should not have.  I thought that providing the
> perspective of one (heavy) user of GDB on this will help make the
> discussion more objective and balanced.  Apologies if that is not
> useful.

I accept your feedback, and concede that it may be interesting to have
a mode that prints a more concise display.  But I find it a different feature.
Nothing prevents you or someone else from adding some "info breakpoints -concise"
option.  It would be up to debate what exactly does "concise" show, e.g., whether that
would mean we print the breakpoint's commands too, for example.  For me, I would like
a flag that only suppresses the locations and nothing else, and that is very easy
to specify (there's no scope for divergence on what "hiding locations" means)
so that's what I am proposing.