[RFA v2/code+DOCO+NEWS] new "set/show serial baud" command

Joel Brobecker
2013-10-09T11:23:43Z

[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]

> I think it's best to also deprecate these aliases.

Unfortunately, I hit a limitation when trying to deprecate the alias,
and my guess is because the command is prefixed. I had a quick look,
and basically, I think "lookup_cmd_1" recurses into himself by first
finding the prefixed command, then does the recursion by trying to
find the rest of the command within that prefix. It finds the command
and notices the fact that it is deprecated, and even tries to warn
the user by calling deprecated_cmd_warning. But this function only
takes the deprecated command names, and the name it passes is lacking
the prefix (due to the recursion). As a result, deprecated_cmd_warning
fails to find the aliased command and returns immediately:

if (!lookup_cmd_composition (text, &alias, &prefix_cmd, &cmd))
/* Return if text doesn't evaluate to a command. */
return;

Lifting that limitation would take a little bit more time than I have,
so I went with the sort of trick as the one used in remote.c, which
is to duplicate the command. Not pretty, but as the FIXME says, it's
temporary (I've already added a TODO item in the GDB 7.8 release
management wiki page).

This patch implements that change, as well as updates the GDB Manual.
For now, it's a straight search-and-replace, as there was no real
section where the option could go. Eventually, I think we will want
to dissociate the options relative to the remote protocol, from the
options specific to serial line handling. I think other option names
might need to be renamed as well, but this I've already far exceeded
the amount of time I could spend on this.

gdb/ChangeLog:

* cli/cli-cmds.c (show_baud_rate): Moved to serial.c as
serial_baud_show_cmd.
(_initialize_cli_cmds): Delete the code creating the
"set/show remotebaud" commands.
* serial.c (baud_rate): Move here from top.c.
(serial_baud_show_cmd): Move here from cli/cli-cmds.c.
(_initialize_serial): Create "set/show serial baud" commands.
Add "set/show remotebaud" command aliases.
* top.c (baud_rate): Moved to serial.c.
* NEWS: Document the new "set/show serial baud" commands,
replacing "set/show remotebaud".

gdb/doc/ChangeLog:

* gdb.texinfo: Replace "set remotebaud" and "show remotebaud"
by "set serial baud" and "show serial baud" (resp) throughout.

I hope it's a sufficient improvement in itself. OK to apply?

--
Joel

[-- Attachment #2: 0001-Rename-set-show-remotebaud-command-into-set-show-ser.patch --]
[-- Type: text/x-diff, Size: 8306 bytes --]

From 7bb83681c2813a3f2cfb0afcd43284f931691bc7 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 8 Oct 2013 17:55:02 +0400
Subject: [PATCH] Rename "set/show remotebaud" command into "set/show serial
baud"

This patch renames the "set/show remotebaud" commands into
"set/show serial baud", and moves its implementation into serial.c.
It also moves the "baud_rate" global from top.c to serial.c, where
the new code is being added (the alternative was to add an include
of target.h).

And to facilitate the transition to the new setting name, this
patch also preserves the old commands, and marks them as deprecated
to alert the users of the change.

gdb/ChangeLog:

* cli/cli-cmds.c (show_baud_rate): Moved to serial.c as
serial_baud_show_cmd.
(_initialize_cli_cmds): Delete the code creating the
"set/show remotebaud" commands.
* serial.c (baud_rate): Move here from top.c.
(serial_baud_show_cmd): Move here from cli/cli-cmds.c.
(_initialize_serial): Create "set/show serial baud" commands.
Add "set/show remotebaud" command aliases.
* top.c (baud_rate): Moved to serial.c.
* NEWS: Document the new "set/show serial baud" commands,
replacing "set/show remotebaud".

gdb/doc/ChangeLog:

* gdb.texinfo: Replace "set remotebaud" and "show remotebaud"
by "set serial baud" and "show serial baud" (resp) throughout.
---
gdb/NEWS | 5 +++++
gdb/cli/cli-cmds.c | 19 ------------------
gdb/doc/gdb.texinfo | 8 ++++----
gdb/serial.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
gdb/top.c | 7 -------
5 files changed, 64 insertions(+), 30 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 8114fb1..4e627b0 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -200,6 +200,11 @@ qXfer:libraries-svr4:read's annex

* GDB can now use Windows x64 unwinding data.

+* The "set remotebaud" command has been replaced by "set serial baud".
+ Similarly, "show remotebaud" has been replaced by "show serial baud".
+ The "set remotebaud" and "show remotebaud" commands are still available
+ to provide backward compatibility with older versions of GDB.
+
*** Changes in GDB 7.6

* Target record has been renamed to record-full.
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 886ba7a..460b719 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -1582,14 +1582,6 @@ show_history_expansion_p (struct ui_file *file, int from_tty,
}

static void
-show_baud_rate (struct ui_file *file, int from_tty,
- struct cmd_list_element *c, const char *value)
-{
- fprintf_filtered (file, _("Baud rate for remote serial I/O is %s.\n"),
- value);
-}
-
-static void
show_remote_debug (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
{
@@ -1748,17 +1740,6 @@ the previous command number shown."),
add_cmd ("configuration", no_set_class, show_configuration,
_("Show how GDB was configured at build time."), &showlist);

- /* If target is open when baud changes, it doesn't take effect until
- the next open (I think, not sure). */
- add_setshow_zinteger_cmd ("remotebaud", no_class, &baud_rate, _("\
-Set baud rate for remote serial I/O."), _("\
-Show baud rate for remote serial I/O."), _("\
-This value is used to set the speed of the serial port when debugging\n\
-using remote targets."),
- NULL,
- show_baud_rate,
- &setlist, &showlist);
-
add_setshow_zinteger_cmd ("remote", no_class, &remote_debug, _("\
Set debugging of remote protocol."), _("\
Show debugging of remote protocol."), _("\
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e196658..bda2e7b 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -18024,8 +18024,8 @@ target remote /dev/ttyb
@end smallexample

If you're using a serial line, you may want to give @value{GDBN} the
-@w{@samp{--baud}} option, or use the @code{set remotebaud} command
-(@pxref{Remote Configuration, set remotebaud}) before the
+@w{@samp{--baud}} option, or use the @code{set serial baud} command
+(@pxref{Remote Configuration, set serial baud}) before the
@code{target} command.

@item target remote @code{@var{host}:@var{port}}
@@ -18567,13 +18567,13 @@ default value is the number of bits in the target's address.
@item show remoteaddresssize
Show the current value of remote address size in bits.

-@item set remotebaud @var{n}
+@item set serial baud @var{n}
@cindex baud rate for remote targets
Set the baud rate for the remote serial I/O to @var{n} baud. The
value is used to set the speed of the serial port used for debugging
remote targets.

-@item show remotebaud
+@item show serial baud
Show the current speed of the remote connection.

@item set remotebreak
diff --git a/gdb/serial.c b/gdb/serial.c
index ee3f1ea..3233e1b 100644
--- a/gdb/serial.c
+++ b/gdb/serial.c
@@ -621,6 +621,20 @@ serial_show_cmd (char *args, int from_tty)
cmd_show_list (serial_show_cmdlist, from_tty, "");
}

+/* Baud rate specified for talking to serial target systems. Default
+ is left as -1, so targets can choose their own defaults. */
+/* FIXME: This means that "show serial baud" and gr_files_info can
+ print -1 or (unsigned int)-1. This is a Bad User Interface. */
+
+int baud_rate = -1;
+
+static void
+serial_baud_show_cmd (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ fprintf_filtered (file, _("Baud rate for remote serial I/O is %s.\n"),
+ value);
+}

void
_initialize_serial (void)
@@ -643,6 +657,47 @@ Show default serial/parallel port configuration."),
0/*allow-unknown*/,
&showlist);

+ /* If target is open when baud changes, it doesn't take effect until
+ the next open (I think, not sure). */
+ add_setshow_zinteger_cmd ("baud", no_class, &baud_rate, _("\
+Set baud rate for remote serial I/O."), _("\
+Show baud rate for remote serial I/O."), _("\
+This value is used to set the speed of the serial port when debugging\n\
+using remote targets."),
+ NULL,
+ serial_baud_show_cmd,
+ &serial_set_cmdlist, &serial_show_cmdlist);
+
+ /* The commands "set/show serial baud" used to have a different name.
+ Add aliases to those names to facilitate the transition, and mark
+ them as deprecated, in order to make users aware of the fact that
+ the command names have been changed. */
+ {
+ const char *cmd_name;
+ struct cmd_list_element *cmd;
+
+ /* FIXME: There is a limitation in the deprecation mechanism,
+ and the warning ends up not being displayed for prefixed
+ aliases. So use a real command instead of an alias.
+ This is temporary code anyway, so should go away soon
+ after the next release cycle starts. */
+ add_setshow_zinteger_cmd ("remotebaud", class_alias, &baud_rate, _("\
+Set baud rate for remote serial I/O."), _("\
+Show baud rate for remote serial I/O."), _("\
+This value is used to set the speed of the serial port when debugging\n\
+using remote targets."),
+ NULL,
+ serial_baud_show_cmd,
+ &setlist, &showlist);
+ cmd_name = "remotebaud";
+ cmd = lookup_cmd (&cmd_name, setlist, "", -1, 1);
+ deprecate_cmd (cmd, "set serial baud");
+ cmd_name
+ = "remotebaud"; /* needed because lookup_cmd updates the pointer */
+ cmd = lookup_cmd (&cmd_name, showlist, "", -1, 1);
+ deprecate_cmd (cmd, "show serial baud");
+ }
+
add_setshow_filename_cmd ("remotelogfile", no_class, &serial_logfile, _("\
Set filename for remote session recording."), _("\
Show filename for remote session recording."), _("\
diff --git a/gdb/top.c b/gdb/top.c
index d9128a3..c473d8c 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -144,13 +144,6 @@ int saved_command_line_size = 100;
is issuing commands too. */
int server_command;

-/* Baud rate specified for talking to serial target systems. Default
- is left as -1, so targets can choose their own defaults. */
-/* FIXME: This means that "show remotebaud" and gr_files_info can
- print -1 or (unsigned int)-1. This is a Bad User Interface. */
-
-int baud_rate = -1;
-
/* Timeout limit for response from target. */

/* The default value has been changed many times over the years. It
--
1.8.1.2