[RFA,7/8] Add truncate_repeat_arguments function
Commit Message
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> I'm rewriting this patch to allow those uses as well... so you might as
Tom> well skip this patch if you're reviewing. However, I think the other
Tom> patches in the series are still ok to read.
Here is a new version of this patch, that addresses the additional
commands I ran across.
Tom
commit 323b5cdc6429d865ddf091913c8b78bb521e20f3
Author: Tom Tromey <tom@tromey.com>
Date: Thu Oct 12 08:27:21 2017 -0600
Add set_repeat_arguments function
The "x", "list", and "show commands" commands have special repetition
behavior: repeating the command doesn't re-run it with the same
arguments
This is currently implemented by modifying the passed-in argument; but
that won't work properly with const arguments (and seems pretty
obscure besides).
This patch adds a new "set_repeat_arguments" function and changes the
relevant places to call it.
ChangeLog
2017-10-15 Tom Tromey <tom@tromey.com>
* printcmd.c (x_command): Call set_repeat_arguments.
* cli/cli-cmds.c (list_command): Call set_repeat_arguments.
* top.c (repeat_arguments): New global.
(set_repeat_arguments): New function.
(execute_command): Handle repeat_arguments.
(show_commands): Calls set_repeat_arguments.
* command.h (set_repeat_arguments): Declare.
Comments
Tom Tromey <tom@tromey.com> writes:
> +/* If not NULL, the arguments that should be passed if the current
> + command is repeated. */
> +
> +static const char *repeat_arguments;
> +
> +/* See command.h. */
> +
> +void
> +set_repeat_arguments (const char *args)
> +{
> + repeat_arguments = args;
> +}
This global variable worries me a little bit. Is it possible that
different part of GDB request to execute the repeatable commands? For
example, CLI and MI issues command, or different UIs issue command.
>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
Yao> This global variable worries me a little bit. Is it possible that
Yao> different part of GDB request to execute the repeatable commands? For
Yao> example, CLI and MI issues command, or different UIs issue command.
It's hard to reason about, I agree.
I think commands can either be run via "cmd_func", or by calling the
command function directly.
Direct calls can be directly audited -- and none of the callers of the
new set_repeat_arguments function are called this way.
There are also not many calls to cmd_func; and at least when I look at
these (I encourage you to check my work), I think only execute_command
can possibly call a command that calls set_repeat_arguments.
Since execute_command uses a scoped_restore to save and restore this
global, I think this must be ok.
It does seem a bit fragile. For example, I'd recommend not adding new
callers of set_repeat_arguments. I'm open to suggestions for another
way to approach this.
Tom
Yao> This global variable worries me a little bit. Is it possible that
Yao> different part of GDB request to execute the repeatable commands? For
Yao> example, CLI and MI issues command, or different UIs issue command.
Tom> It's hard to reason about, I agree.
[...]
I think this is the last remaining constification patch of mine that
requires approval. There weren't any concrete suggestions coming from
this exchange; but if you have one I am happy to try to implement it.
thanks,
Tom
@@ -1,5 +1,15 @@
2017-10-15 Tom Tromey <tom@tromey.com>
+ * printcmd.c (x_command): Call set_repeat_arguments.
+ * cli/cli-cmds.c (list_command): Call set_repeat_arguments.
+ * top.c (repeat_arguments): New global.
+ (set_repeat_arguments): New function.
+ (execute_command): Handle repeat_arguments.
+ (show_commands): Calls set_repeat_arguments.
+ * command.h (set_repeat_arguments): Declare.
+
+2017-10-15 Tom Tromey <tom@tromey.com>
+
* stack.c (backtrace_command): Use std::string.
(backtrace_command_1): Make "count_exp" const.
@@ -1082,7 +1082,7 @@ list_command (char *arg, int from_tty)
turn it into the no-arg variant. */
if (from_tty)
- *arg = 0;
+ set_repeat_arguments ("");
if (dummy_beg && sal_end.symtab == 0)
error (_("No default source file yet. Do \"help list\"."));
@@ -445,6 +445,11 @@ extern void dont_repeat (void);
extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
+/* Set the arguments that will be passed if the current command is
+ repeated. Note that the passed-in string must be a constant. */
+
+extern void set_repeat_arguments (const char *args);
+
/* Used to mark commands that don't do anything. If we just leave the
function field NULL, the command is interpreted as a help topic, or
as a class of commands. */
@@ -1636,7 +1636,7 @@ x_command (char *exp, int from_tty)
repeated with Newline. But don't clobber a user-defined
command's definition. */
if (from_tty)
- *exp = 0;
+ set_repeat_arguments ("");
val = evaluate_expression (expr.get ());
if (TYPE_IS_REFERENCE (value_type (val)))
val = coerce_ref (val);
@@ -528,6 +528,19 @@ maybe_wait_sync_command_done (int was_sync)
wait_sync_command_done ();
}
+/* If not NULL, the arguments that should be passed if the current
+ command is repeated. */
+
+static const char *repeat_arguments;
+
+/* See command.h. */
+
+void
+set_repeat_arguments (const char *args)
+{
+ repeat_arguments = args;
+}
+
/* Execute the line P as a command, in the current user context.
Pass FROM_TTY as second argument to the defining function. */
@@ -571,6 +584,10 @@ execute_command (char *p, int from_tty)
c = lookup_cmd (&cmd, cmdlist, "", 0, 1);
p = (char *) cmd;
+ scoped_restore save_repeat_args
+ = make_scoped_restore (&repeat_arguments, nullptr);
+ char *args_pointer = p;
+
/* Pass null arg rather than an empty one. */
arg = *p ? p : 0;
@@ -619,6 +636,11 @@ execute_command (char *p, int from_tty)
/* If this command has been post-hooked, run the hook last. */
execute_cmd_post_hook (c);
+ if (repeat_arguments != NULL)
+ {
+ gdb_assert (strlen (args_pointer) >= strlen (repeat_arguments));
+ strcpy (args_pointer, repeat_arguments);
+ }
}
check_frame_language_change ();
@@ -1675,7 +1697,7 @@ dont_repeat_command (char *ignored, int from_tty)
/* Number of commands to print in each call to show_commands. */
#define Hist_print 10
void
-show_commands (char *args, int from_tty)
+show_commands (const char *args, int from_tty)
{
/* Index for history commands. Relative to history_base. */
int offset;
@@ -1729,10 +1751,7 @@ show_commands (char *args, int from_tty)
"show commands +" does. This is unnecessary if arg is null,
because "show commands +" is not useful after "show commands". */
if (from_tty && args)
- {
- args[0] = '+';
- args[1] = '\0';
- }
+ set_repeat_arguments ("+");
}
/* Update the size of our command history file to HISTORY_SIZE.
@@ -2005,10 +2005,7 @@ show_values (char *num_exp, int from_tty)
"show values +". If num_exp is null, this is unnecessary, since
"show values +" is not useful after "show values". */
if (from_tty && num_exp)
- {
- num_exp[0] = '+';
- num_exp[1] = '\0';
- }
+ set_repeat_arguments ("+");
}
enum internalvar_kind