This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
- From: Pedro Alves <palves at redhat dot com>
- To: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>, gdb-patches at sourceware dot org
- Date: Fri, 1 Jun 2018 15:32:17 +0100
- Subject: Re: [RFA 0/8] Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
- References: <20180521110651.13842-1-philippe.waroquiers@skynet.be>
Hi Philippe,
I really wish I had time to play a bit more with the series
(I really like the idea of "frame apply") and do a more in-depth
review today, but I probably won't, so here are some quick comments.
On 05/21/2018 12:06 PM, Philippe Waroquiers wrote:
> Implement 'frame apply COMMAND', enhance 'thread apply COMMAND'
>
> Compared to RFC, this handles all comments received from Eli and Simon,
> and completes the changes so that it is (should be) ready for RFA.
>
> This patch series :
> * implements a new command
> 'frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND'.
> * enhance 'thread apply COMMAND' by adding a -FLAGS argument
> * adds some shortcuts commands
> * documents the above in gdb.texinfo and NEWS.
> * adds a unit test for cli-utils.c
> * adds test for 'frame apply'
> * modify gdb.threads/pthreads.exp to test 'thread apply' -FLAGS argument
I'm not sure the idea of using "-" for flags is a good one,
because that conflicts with GDB's usual use of "-" for long
options, which can be abbreviated, and cannot be combined.
For example, "watch -location", "watch -l".
A while ago I was playing with adding a generic framework for
command options, which also handled TAB completion
automatically, and I was thinking about how gdb doesn't use "--"
for long options unlike getopt, and how single-"-" for long options
prevent combining options with a single "-", like you can
with "ls --all --size" -> "ls -as". Then I realized something that I
had haven't seen written down, but I thought made some sense. That
is, that we do have at least one command that allows combining short
options, "x/FMT", and it just uses "/" instead of "-" I.e., we
could make that the way to handle short vs long options throughout.
I.e., comparing gdb's options to getopt-like options yields this:
| getopt | gdb |
long | -- | - |
short | - | / |
So I'm wondering about using / for these new flags too.
Like, long form "-verbose -continue", short form "/vc".
Or you could sidestep the issue by ditching support for
combining flags, i.e., require "-v -v -c" instead of "-vvc".
>
> Th new command 'frame apply' allows to apply a COMMAND to a number of frames,
> or to all frames.
> The optional -FLAGS... argument allows to control what output to produce
> and how to handle errors raised when applying COMMAND to a frame.
>
> Some examples usages for this new command:
> frame apply all info frame
> Produce info frame for all frames
> frame apply all p $sp
> For each frame, print the location, followed by the frame sp
> frame apply all -qq p $sp
> Same as before, but -qq flags (q = quiet) indicate to only print
> the frames sp.
> frame apply all -vv p $sp
> Same as before, but -vv flags (v = verbose) indicate to print
> location and source line for each frame.
> frame apply all p some_local_var_somewhere
> Print some_local_var_somewhere in all frames. 'frame apply'
> will abort as soon as the print command fails.
> frame apply all -c p some_local_var_somewhere
> Same as before, but -c flag (c = continue) means to
> print the error and continue applying command in case the
> print command fails.
> frame apply all -s p some_local_var_somewhere
> Same as before, but -s flag (s = silent) means to
> be silent for frames where the print command fails.
> In other words, this allows to 'search' the frame in which
> some_local_var_somewhere can be printed.
>
> 'thread apply' command has been enhanced to also accepts a -FLAGS...
> argument.
>
> Some examples usages for this new argument:
> thread apply all -s frame apply all -s p some_local_var_somewhere
> Prints the thread id, frame location and some_local_var_somewhere
> value in frames of threads that have such local var.
>
> To make the life of the user easier, the most typical use cases
> have shortcuts :
> faas : shortcut for 'frame apply all -s'
> taas : shortcut for 'thread apply all -s'
> tfaas : shortcut for 'thread apply all -s frame apply all -s"
I'm not particularly sold on adding aliases, since you can
abbreviate and tab-complete. Users are used to "t a a",
for example, so I think "f a a" will come naturally, and
users can add aliases themselves with the "alias" command.
But that may be because I haven't played with the patches much yet.
> An example usage :
> tfaas p some_local_var_somewhere
> same as the longer:
> 'thread apply all -s frame apply all -s p some_local_var_somewhere'
>
> gdb/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
> gdb/testsuite/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
> gdb/doc/ChangeLog
> 2018-05-21 Philippe Waroquiers <philippe.waroquiers@skynet.be>
>
Is the idea that the patches should be merged as a single bigger
patch? Otherwise, the ChangeLogs should be split into the
individual patches.
Thanks,
Pedro Alves