[RFA,v3,01/13] Rationalize "backtrace" command line parsing

Message ID 20180323205512.14434-2-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey March 23, 2018, 8:55 p.m. UTC
  The backtrace command has peculiar command-line parsing.  In
particular, it splits the command line, then loops over the arguments.
If it sees a word it recognizes, like "full", it effectively drops
this word from the argument vector.  Then, it pastes together the
remaining arguments, passing them on to backtrace_command_1, which in
turn passes the resulting string to parse_and_eval_long.

The documentation doesn't mention the parse_and_eval_long at all, so
it is a bit of a hidden feature that you can "bt 3*2".  The strange
algorithm above also means you can "bt 3 * no-filters 2" and get 6
frames...

This patch changes backtrace's command line parsing to be a bit more
rational.  Now, special words like "full" are only recognized at the
start of the command.

This also updates the documentation to describe the various bt options
individually.

gdb/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

	* stack.c (backtrace_command): Rewrite command line parsing.

gdb/doc/ChangeLog
2018-03-23  Tom Tromey  <tom@tromey.com>

	* gdb.texinfo (Backtrace): Describe options individually.
---
 gdb/ChangeLog       |  4 ++++
 gdb/doc/ChangeLog   |  4 ++++
 gdb/doc/gdb.texinfo | 54 ++++++++++++++++++++--------------------------
 gdb/stack.c         | 62 +++++++++++++++++------------------------------------
 4 files changed, 51 insertions(+), 73 deletions(-)
  

Comments

Eli Zaretskii March 24, 2018, 6:31 a.m. UTC | #1
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>
> Date: Fri, 23 Mar 2018 14:55:00 -0600
> 
> The backtrace command has peculiar command-line parsing.  In
> particular, it splits the command line, then loops over the arguments.
> If it sees a word it recognizes, like "full", it effectively drops
> this word from the argument vector.  Then, it pastes together the
> remaining arguments, passing them on to backtrace_command_1, which in
> turn passes the resulting string to parse_and_eval_long.
> 
> The documentation doesn't mention the parse_and_eval_long at all, so
> it is a bit of a hidden feature that you can "bt 3*2".  The strange
> algorithm above also means you can "bt 3 * no-filters 2" and get 6
> frames...
> 
> This patch changes backtrace's command line parsing to be a bit more
> rational.  Now, special words like "full" are only recognized at the
> start of the command.
> 
> This also updates the documentation to describe the various bt options
> individually.
> 
> gdb/ChangeLog
> 2018-03-23  Tom Tromey  <tom@tromey.com>
> 
> 	* stack.c (backtrace_command): Rewrite command line parsing.
> 
> gdb/doc/ChangeLog
> 2018-03-23  Tom Tromey  <tom@tromey.com>
> 
> 	* gdb.texinfo (Backtrace): Describe options individually.
> ---
>  gdb/ChangeLog       |  4 ++++
>  gdb/doc/ChangeLog   |  4 ++++
>  gdb/doc/gdb.texinfo | 54 ++++++++++++++++++++--------------------------
>  gdb/stack.c         | 62 +++++++++++++++++------------------------------------
>  4 files changed, 51 insertions(+), 73 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 74e0fdb4a4..b48dd4ed4b 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -7307,39 +7307,31 @@ frame (frame zero), followed by its caller (frame one), and on up the
>  stack.
>  
>  @anchor{backtrace-command}
> -@table @code
>  @kindex backtrace
>  @kindex bt @r{(@code{backtrace})}
> -@item backtrace
> -@itemx bt
> -Print a backtrace of the entire stack: one line per frame for all
> -frames in the stack.
> -
> -You can stop the backtrace at any time by typing the system interrupt
> -character, normally @kbd{Ctrl-c}.
> -
> -@item backtrace @var{n}
> -@itemx bt @var{n}
> -Similar, but print only the innermost @var{n} frames.
> -
> -@item backtrace -@var{n}
> -@itemx bt -@var{n}
> -Similar, but print only the outermost @var{n} frames.
> -
> -@item backtrace full
> -@itemx bt full
> -@itemx bt full @var{n}
> -@itemx bt full -@var{n}
> -Print the values of the local variables also.  As described above,
> -@var{n} specifies the number of frames to print.
> -
> -@item backtrace no-filters
> -@itemx bt no-filters
> -@itemx bt no-filters @var{n}
> -@itemx bt no-filters -@var{n}
> -@itemx bt no-filters full
> -@itemx bt no-filters full @var{n}
> -@itemx bt no-filters full -@var{n}

Is it wise to delete the @table?  We always describe commands in that
format, AFAIR.

> +Print a backtrace of the entire stack, use the @code{backtrace}
> +command, or its alias @code{bt}.  This command will print one line per

I gues you meant "To print a backtrace of the entire stack ...",
because otherwise the first sentence reads weirdly.

Otherwise, the documentation part is OK.  Thanks.
  
Tom Tromey March 25, 2018, 4:50 p.m. UTC | #2
>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> Is it wise to delete the @table?  We always describe commands in that
Eli> format, AFAIR.

The patch actually just moves the table down a bit and removes some text
from each entry.  It isn't really deleted.  For example:

-@item backtrace @var{n}
-@itemx bt @var{n}
-Similar, but print only the innermost @var{n} frames.

becomes

+@table @code
+@item @var{n}
+@itemx @var{n}
+Print only the innermost @var{n} frames, where @var{n} is a positive
+number.

What here would you like changed?

>> +Print a backtrace of the entire stack, use the @code{backtrace}
>> +command, or its alias @code{bt}.  This command will print one line per

Eli> I gues you meant "To print a backtrace of the entire stack ...",
Eli> because otherwise the first sentence reads weirdly.

I'll make this change.

Tom
  
Eli Zaretskii March 25, 2018, 5:11 p.m. UTC | #3
> From: Tom Tromey <tom@tromey.com>
> Cc: Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Sun, 25 Mar 2018 10:50:52 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> Is it wise to delete the @table?  We always describe commands in that
> Eli> format, AFAIR.
> 
> The patch actually just moves the table down a bit and removes some text
> from each entry.  It isn't really deleted.  For example:
> 
> -@item backtrace @var{n}
> -@itemx bt @var{n}
> -Similar, but print only the innermost @var{n} frames.
> 
> becomes
> 
> +@table @code
> +@item @var{n}
> +@itemx @var{n}
> +Print only the innermost @var{n} frames, where @var{n} is a positive
> +number.

Yes, but the original table included the command, as we do with all
commands, whereas your new table includes only the various forms of
arguments to the command, and the command itself is never mentioned,
except in the surrounding text.

> What here would you like changed?

I expected to see something like this, before the description of
arguments:

 @table @code
 @item backtrace [@var{args}@dots{}]
 @itemx bt [@var{args}@dots{}]
 Print the backtrace of the entire stack.  The optional @var{args} can
 be one of the following:

followed by your table of arguments.

Thanks.
  

Patch

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 74e0fdb4a4..b48dd4ed4b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7307,39 +7307,31 @@  frame (frame zero), followed by its caller (frame one), and on up the
 stack.
 
 @anchor{backtrace-command}
-@table @code
 @kindex backtrace
 @kindex bt @r{(@code{backtrace})}
-@item backtrace
-@itemx bt
-Print a backtrace of the entire stack: one line per frame for all
-frames in the stack.
-
-You can stop the backtrace at any time by typing the system interrupt
-character, normally @kbd{Ctrl-c}.
-
-@item backtrace @var{n}
-@itemx bt @var{n}
-Similar, but print only the innermost @var{n} frames.
-
-@item backtrace -@var{n}
-@itemx bt -@var{n}
-Similar, but print only the outermost @var{n} frames.
-
-@item backtrace full
-@itemx bt full
-@itemx bt full @var{n}
-@itemx bt full -@var{n}
-Print the values of the local variables also.  As described above,
-@var{n} specifies the number of frames to print.
-
-@item backtrace no-filters
-@itemx bt no-filters
-@itemx bt no-filters @var{n}
-@itemx bt no-filters -@var{n}
-@itemx bt no-filters full
-@itemx bt no-filters full @var{n}
-@itemx bt no-filters full -@var{n}
+Print a backtrace of the entire stack, use the @code{backtrace}
+command, or its alias @code{bt}.  This command will print one line per
+frame for frames in the stack.  By default, all stack frames are
+printed.  You can stop the backtrace at any time by typing the system
+interrupt character, normally @kbd{Ctrl-c}.  @code{backtrace} can
+accept some arguments:
+
+@table @code
+@item @var{n}
+@itemx @var{n}
+Print only the innermost @var{n} frames, where @var{n} is a positive
+number.
+
+@item -@var{n}
+@itemx -@var{n}
+Print only the outermost @var{n} frames, where @var{n} is a positive
+number.
+
+@item full
+Print the values of the local variables also.  This can be combined
+with a number to limit the number of frames shown.
+
+@item no-filters
 Do not run Python frame filters on this backtrace.  @xref{Frame
 Filter API}, for more information.  Additionally use @ref{disable
 frame-filter all} to turn off all frame filters.  This is only
diff --git a/gdb/stack.c b/gdb/stack.c
index aad8fcd987..13af6594a9 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1850,61 +1850,39 @@  backtrace_command_1 (const char *count_exp, int show_locals, int no_filters,
 static void
 backtrace_command (const char *arg, int from_tty)
 {
-  int fulltrace_arg = -1, arglen = 0, argc = 0, no_filters  = -1;
-  int user_arg = 0;
+  bool fulltrace = false;
+  bool filters = true;
 
-  std::string reconstructed_arg;
   if (arg)
     {
-      char **argv;
-      int i;
+      bool done = false;
 
-      gdb_argv built_argv (arg);
-      argv = built_argv.get ();
-      argc = 0;
-      for (i = 0; argv[i]; i++)
+      while (!done)
 	{
-	  unsigned int j;
+	  const char *save_arg = arg;
+	  std::string this_arg = extract_arg (&arg);
 
-	  for (j = 0; j < strlen (argv[i]); j++)
-	    argv[i][j] = TOLOWER (argv[i][j]);
+	  if (this_arg.empty ())
+	    break;
 
-	  if (no_filters < 0 && subset_compare (argv[i], "no-filters"))
-	    no_filters = argc;
+	  if (subset_compare (this_arg.c_str (), "no-filters"))
+	    filters = false;
+	  else if (subset_compare (this_arg.c_str (), "full"))
+	    fulltrace = true;
 	  else
 	    {
-	      if (fulltrace_arg < 0 && subset_compare (argv[i], "full"))
-		fulltrace_arg = argc;
-	      else
-		{
-		  user_arg++;
-		  arglen += strlen (argv[i]);
-		}
-	    }
-	  argc++;
-	}
-      arglen += user_arg;
-      if (fulltrace_arg >= 0 || no_filters >= 0)
-	{
-	  if (arglen > 0)
-	    {
-	      for (i = 0; i < argc; i++)
-		{
-		  if (i != fulltrace_arg && i != no_filters)
-		    {
-		      reconstructed_arg += argv[i];
-		      reconstructed_arg += " ";
-		    }
-		}
-	      arg = reconstructed_arg.c_str ();
+	      /* Not a recognized argument, so stop.  */
+	      arg = save_arg;
+	      done = true;
 	    }
-	  else
-	    arg = NULL;
 	}
+
+      if (*arg == '\0')
+	arg = NULL;
     }
 
-  backtrace_command_1 (arg, fulltrace_arg >= 0 /* show_locals */,
-		       no_filters >= 0 /* no frame-filters */, from_tty);
+  backtrace_command_1 (arg, fulltrace /* show_locals */,
+		       !filters /* no frame-filters */, from_tty);
 }
 
 /* Iterate over the local variables of a block B, calling CB with