[review v4] [Debugging output] Make remote packet truncation length adjustable

Pedro Alves (Code Review) gerrit@gnutoolchain-gerrit.osci.io
Thu Nov 21 18:55:00 GMT 2019


Pedro Alves has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/691
......................................................................


Patch Set 4:

(5 comments)

Thanks.  A few comments more.

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +19,19 @@ gdb/ChangeLog:
| +2019-11-21  Luis Machado  <luis.machado@linaro.org>
| +
| +	* NEWS (New Commands): Mention "set debug remote-packet-max-chars".
| +	* remote.c (REMOTE_DEBUG_MAX_CHAR): Remove.
| +	(remote_packet_max_chars): New static global.
| +	(show_remote_packet_max_chars): New function.
| +	(remote_target::putpkt_binary): Adjust to use new
| +	remote_packet_max_chars option.
| +	(remote_target::getpkt_or_notif_sane_1): Likewise.
| +	(_initialize_remote): Register new remote_packet_max_chars option.

PS4, Line 28:

remote_packet_max_chars -> remote-packet-max-chars

| +
| +gdb/doc/ChangeLog:
| +
| +2019-11-21  Luis Machado  <luis.machado@linaro.org>
| +
| +	* gdb.texinfo (Debugging Output): Document set debug
| +	remote-packet-max-chars.
| +
| +Signed-off-by: Luis Machado <luis.machado@linaro.org>
| --- gdb/NEWS
| +++ gdb/NEWS
| @@ -183,14 +183,19 @@ info module variables [-q] [-m MODULE_REGEXP] [-t TYPE_REGEXP] [REGEXP]
|    Return a list of variables within all modules, grouped by module.
|    The list of variables can be restricted with the optional regular
|    expressions.  MODULE_REGEXP matches against the module name,
|    TYPE_REGEXP matches against the variable type, and REGEXP matches
|    against the variable name.
|  
| +set debug remote-packet-max-chars
| +show debug remote-packet-max-chars
| +  Controls the amount of characters to output when using "set debug remote".
| +  The default is 512 bytes.

PS4, Line 192:

Should mention "packet" somewhere in this description.  Maybe just
"amount of packet characters".

| +
|  * Changed commands
|  
|  help
|    The "help" command uses the title style to enhance the
|    readibility of its output by styling the classes and
|    command names.
|  
|  apropos [-v] REGEXP
| --- gdb/remote.c
| +++ gdb/remote.c
| @@ -1037,19 +1037,17 @@ /* For "set remote" and "show remote".  */
|  static struct cmd_list_element *remote_set_cmdlist;
|  static struct cmd_list_element *remote_show_cmdlist;
|  
|  /* Controls whether GDB is willing to use range stepping.  */
|  
|  static bool use_range_stepping = true;
|  
|  /* The max number of chars in debug output.  The rest of chars are
|     omitted.  */
|  

PS4, Line 1046:

This comment above...

| -#define REMOTE_DEBUG_MAX_CHAR 512
| -
|  /* Private data that we'll store in (struct thread_info)->priv.  */
|  struct remote_thread_info : public private_thread_info
|  {
|    std::string extra;
|    std::string name;
|    int core = -1;
|  

 ...

| @@ -1706,16 +1704,30 @@ /* Show the number of hardware breakpoints that can be used.  */
|  static void
|  show_hardware_breakpoint_limit (struct ui_file *file, int from_tty,
|  				struct cmd_list_element *c,
|  				const char *value)
|  {
|    fprintf_filtered (file, _("The maximum number of target hardware "
|  			    "breakpoints is %s.\n"), value);
|  }
|  
| +static int remote_packet_max_chars = 512;

PS4, Line 1713:

... should be moved here.

| +
| +/* Show the maximum number of characters to display for a remote packet when
| +   remote debugging is enabled.  */
| +
| +static void
| +show_remote_packet_max_chars (struct ui_file *file, int from_tty,
| +			      struct cmd_list_element *c,
| +			      const char *value)
| +{
| +  fprintf_filtered (file, _("Remote packet output will be truncated at %s "
| +			    "characters.\n"), value);

PS4, Line 1724:

Pedantically, this string doesn't work well with singular / 1
character.
Come to think of it, nor with "unlimited"?  Does it say:

  "Remote packet output will be truncated at unlimited characters."

Better rephrase that as:

  "something something something is %s.\n", value

| +}
| +
|  long
|  remote_target::get_memory_write_packet_size ()
|  {
|    return get_memory_packet_size (&memory_write_packet_config);
|  }
|  
|  static struct memory_packet_config memory_read_packet_config =

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I2e871b37bfcaa6376537c3fe3db8f016dd806a7c
Gerrit-Change-Number: 691
Gerrit-PatchSet: 4
Gerrit-Owner: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Luis Machado <luis.machado@linaro.org>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-CC: Pedro Alves <palves@redhat.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 18:54:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment



More information about the Gdb-patches mailing list