Re: [PATCH v5 1/4] Make linux checkpoints work with multiple inferiors

[Kevin Buettner <kevinb@redhat.com>]

On Thu, 30 Jan 2025 17:45:25 +0000
Andrew Burgess <aburgess@redhat.com> wrote:

> Additionally, there are a couple of places in this patch where you have
> spaces instead of tabs, and trailing whitespace.  I have this in my
> ~/.gitconfig file:
> 
>   [alias]
>           check=show --check --format="%C(auto)%h\\ %s\\ %C(blue)[%an\\ (%cr)]%C(reset)"
> 
> Then you can do `git check` to check the last commit, or `git check SHA`
> to check some other commit.  The output will highlight any whitespace
> issues with the patch.

Thanks!  I've added those lines to my own ~/.gitconfig file and will use
"git check" going forward.

[...]
> > Before:
> >
> > (gdb) info checkpoints
> > * 0 Thread 0x7ffff7cd3740 (LWP 84201) (main process) at 0x40114a, file hello.c, line 28
> >   1 process 84205 at 0x401199, file hello.c, line 51
> >   2 process 84206 at 0x4011a3, file hello.c, line 53
> >
> > After:
> >
> > (gdb) info checkpoints
> >   Id Active Target Id      Frame
> > *  0 y      process 551311 at 0x40114a, file hello.c, line 28
> >    1 n      process 551314 at 0x401199, file hello.c, line 51
> >    2 n      process 551315 at 0x4011a3, file hello.c, line 53  
> 
> I'd like to suggest using GDB's structured table generation mechanism
> here.  There's two great examples; 'print_inferior' in inferior.c (which
> does the 'info inferiors' command) and 'print_thread_info_1' in thread.c
> (which is the 'info threads' command).
> 
> I'd also like to suggest, that for the 'Frame' column, you make use of
> print_stack_frame().  This is the function that 'info threads' uses to
> print its stack frame, I think having consistent output would be a win
> here.
> 
> That all said, if you wanted to merge this now, and then we could
> possibly come back and tweak the output formatting and content
> afterwards, I'd be fine with that too.
> 
> Either way:
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks for the review!

I've pushed this series.  (Though I did first fix the whitespace problems,
made sure that things still built and checked that the affected tests still ran
as expected.)

I'll work on a separate patch to use GDB's mechanism for structured
table generation.  (I'll also incorporate your print_stack_frame()
suggestion.) Thanks for the pointers!

Kevin