Handle "show remote memory-write-packet-size" when not connected
authorPedro Alves <palves@redhat.com>
Tue, 22 May 2018 17:22:10 +0000 (18:22 +0100)
committerPedro Alves <palves@redhat.com>
Tue, 22 May 2018 17:34:03 +0000 (18:34 +0100)
Currently "show remote memory-write-packet-size" says that the packet
size is limited to whatever is stored in the remote_state global, even
if not connected to a target.

When we get to support multiple instances of remote targets, there
won't be a remote_state global anymore, so that must be replaced by
something else.

Since it doesn't make sense to print the limit of the packet size of a
non-existing connection, this patch makes us say that the limit will
be further reduced when we connect.

The text is taken from the command's online help, which says:

 "The actual limit is further reduced dependent on the target."

Note that a value of "0" is special, as per "help set remote
memory-write-packet-size":

 ~~~
 Specify the number of bytes in a packet or 0 (zero) for the
 default packet size.
 ~~~

I've tweaked "show remote memory-write-packet-size" to include
"(default)" in the output in that case, like this:

 (gdb) show remote memory-write-packet-size
 The memory-write-packet-size is 0 (default). The actual limit will be further reduced dependent on the target.

While working on this, I noticed that an explicit "set remote
write-packet-size 0" does not makes GDB go back to the exact same
state as the default state when GDB starts up:

 (gdb) show remote memory-write-packet-size
 The memory-write-packet-size is 0. [...]
                                 ^^

 (gdb) set remote memory-write-packet-size 0
 (gdb) show remote memory-write-packet-size
 The memory-write-packet-size is 16384. [...]
                                 ^^^^^

The "16384" number comes from DEFAULT_MAX_MEMORY_PACKET_SIZE.

This happens because git commit a5c0808e221c ("gdb: remove packet size
limit") at
<https://sourceware.org/ml/gdb-patches/2015-08/msg00743.html>, added
this:

  /* So that the query shows the correct value.  */
  if (size <= 0)
    size = DEFAULT_MAX_MEMORY_PACKET_SIZE;

to set_memory_packet_size, but despite what the comment suggests, that
also has the side-effect of recording DEFAULT_MAX_MEMORY_PACKET_SIZE
in config->size.

Finally, DEFAULT_MAX_MEMORY_PACKET_SIZE only makes sense for "set
remote memory-write-packet-size fixed", so I've renamed it
accordingly, to make it a little bit clearer.

gdb/ChangeLog:
2018-05-22  Pedro Alves  <palves@redhat.com>

* remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE): Rename to ...
(DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED): ... this.
(get_fixed_memory_packet_size): New.
(get_memory_packet_size): Use it.
(set_memory_packet_size): Don't override the config size with
DEFAULT_MAX_MEMORY_PACKET_SIZE.
(show_memory_packet_size): Use get_fixed_memory_packet_size.
Don't refer to get_memory_packet_size if not connected to a remote
target.  Show "(default)" if configured size is 0.

gdb/testsuite/ChangeLog:
2018-05-22  Pedro Alves  <palves@redhat.com>

* gdb.base/remote.exp: Adjust expected output of "show remote
memory-write-packet-size".  Add tests for "set remote
memory-write-packet-size 0" and "set remote
memory-write-packet-size fixed/limit".

gdb/ChangeLog
gdb/remote.c
gdb/testsuite/ChangeLog
gdb/testsuite/gdb.base/remote.exp

index 55afb54..3504a3c 100644 (file)
@@ -1,5 +1,17 @@
 2018-05-22  Pedro Alves  <palves@redhat.com>
 
+       * remote.c (DEFAULT_MAX_MEMORY_PACKET_SIZE): Rename to ...
+       (DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED): ... this.
+       (get_fixed_memory_packet_size): New.
+       (get_memory_packet_size): Use it.
+       (set_memory_packet_size): Don't override the config size with
+       DEFAULT_MAX_MEMORY_PACKET_SIZE.
+       (show_memory_packet_size): Use get_fixed_memory_packet_size.
+       Don't refer to get_memory_packet_size if not connected to a remote
+       target.  Show "(default)" if configured size is 0.
+
+2018-05-22  Pedro Alves  <palves@redhat.com>
+
        * remote.c (remote_target::mourn_inferior): Move
        discard_pending_stop_replies call here from ...
        (_initialize_remote): ... here.
index 59880a9..72254db 100644 (file)
@@ -1328,16 +1328,29 @@ struct memory_packet_config
   int fixed_p;
 };
 
-/* The default max memory-write-packet-size.  The 16k is historical.
-   (It came from older GDB's using alloca for buffers and the
-   knowledge (folklore?) that some hosts don't cope very well with
-   large alloca calls.)  */
-#define DEFAULT_MAX_MEMORY_PACKET_SIZE 16384
+/* The default max memory-write-packet-size, when the setting is
+   "fixed".  The 16k is historical.  (It came from older GDB's using
+   alloca for buffers and the knowledge (folklore?) that some hosts
+   don't cope very well with large alloca calls.)  */
+#define DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED 16384
 
 /* The minimum remote packet size for memory transfers.  Ensures we
    can write at least one byte.  */
 #define MIN_MEMORY_PACKET_SIZE 20
 
+/* Get the memory packet size, assuming it is fixed.  */
+
+static long
+get_fixed_memory_packet_size (struct memory_packet_config *config)
+{
+  gdb_assert (config->fixed_p);
+
+  if (config->size <= 0)
+    return DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED;
+  else
+    return config->size;
+}
+
 /* Compute the current size of a read/write packet.  Since this makes
    use of ``actual_register_packet_size'' the computation is dynamic.  */
 
@@ -1349,12 +1362,7 @@ get_memory_packet_size (struct memory_packet_config *config)
 
   long what_they_get;
   if (config->fixed_p)
-    {
-      if (config->size <= 0)
-       what_they_get = DEFAULT_MAX_MEMORY_PACKET_SIZE;
-      else
-       what_they_get = config->size;
-    }
+    what_they_get = get_fixed_memory_packet_size (config);
   else
     {
       what_they_get = get_remote_packet_size ();
@@ -1414,16 +1422,17 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config)
         something arbitrarily large.  */
     }
 
-  /* So that the query shows the correct value.  */
-  if (size <= 0)
-    size = DEFAULT_MAX_MEMORY_PACKET_SIZE;
-
   /* Extra checks?  */
   if (fixed_p && !config->fixed_p)
     {
+      /* So that the query shows the correct value.  */
+      long query_size = (size <= 0
+                        ? DEFAULT_MAX_MEMORY_PACKET_SIZE_FIXED
+                        : size);
+
       if (! query (_("The target may not be able to correctly handle a %s\n"
                   "of %ld bytes. Change the packet size? "),
-                  config->name, size))
+                  config->name, query_size))
        error (_("Packet size not changed."));
     }
   /* Update the config.  */
@@ -1434,13 +1443,24 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config)
 static void
 show_memory_packet_size (struct memory_packet_config *config)
 {
-  printf_filtered (_("The %s is %ld. "), config->name, config->size);
+  if (config->size == 0)
+    printf_filtered (_("The %s is 0 (default). "), config->name);
+  else
+    printf_filtered (_("The %s is %ld. "), config->name, config->size);
   if (config->fixed_p)
     printf_filtered (_("Packets are fixed at %ld bytes.\n"),
-                    get_memory_packet_size (config));
+                    get_fixed_memory_packet_size (config));
   else
-    printf_filtered (_("Packets are limited to %ld bytes.\n"),
-                    get_memory_packet_size (config));
+    {
+      struct remote_state *rs = get_remote_state ();
+
+      if (rs->remote_desc != NULL)
+       printf_filtered (_("Packets are limited to %ld bytes.\n"),
+                        get_memory_packet_size (config));
+      else
+       puts_filtered ("The actual limit will be further reduced "
+                      "dependent on the target.\n");
+    }
 }
 
 static struct memory_packet_config memory_write_packet_config =
index 393ab88..25df82b 100644 (file)
@@ -1,5 +1,12 @@
 2018-05-22  Pedro Alves  <palves@redhat.com>
 
+       * gdb.base/remote.exp: Adjust expected output of "show remote
+       memory-write-packet-size".  Add tests for "set remote
+       memory-write-packet-size 0" and "set remote
+       memory-write-packet-size fixed/limit".
+
+2018-05-22  Pedro Alves  <palves@redhat.com>
+
        PR gdb/22973
        * gdb.base/utf8-identifiers.c: New file.
        * gdb.base/utf8-identifiers.exp: New file.
index 26361af..ba34441 100644 (file)
@@ -35,7 +35,7 @@ if {$result != "" } then {
 #
 
 gdb_test "show remote memory-write-packet-size" \
-       "The memory-write-packet-size is 0. Packets are limited to \[0-9\]+ bytes." \
+       "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
        "write-packet default"
 
 gdb_test "set remote memory-write-packet-size" \
@@ -44,14 +44,38 @@ gdb_test "set remote memory-write-packet-size" \
 
 gdb_test_no_output "set remote memory-write-packet-size 20"
 gdb_test "show remote memory-write-packet-size" \
-       "The memory-write-packet-size is 20. Packets are limited to 20 bytes." \
+       "The memory-write-packet-size is 20. The actual limit will be further reduced dependent on the target\." \
        "set write-packet - small"
 
 gdb_test_no_output "set remote memory-write-packet-size 1"
 gdb_test "show remote memory-write-packet-size" \
-       "The memory-write-packet-size is 1. Packets are limited to 20 bytes." \
+       "The memory-write-packet-size is 1. The actual limit will be further reduced dependent on the target\." \
        "set write-packet - very-small"
 
+gdb_test_no_output "set remote memory-write-packet-size 0"
+gdb_test "show remote memory-write-packet-size" \
+       "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
+       "write-packet default again"
+
+set test "set remote memory-write-packet-size fixed"
+gdb_test_multiple $test $test {
+    -re "Change the packet size. .y or n. " {
+       gdb_test_multiple "y" $test {
+           -re "$gdb_prompt $" {
+               pass $test
+           }
+       }
+    }
+}
+gdb_test "show remote memory-write-packet-size" \
+       "The memory-write-packet-size is 0 \\(default\\). Packets are fixed at 16384 bytes\." \
+       "write-packet default fixed"
+
+gdb_test_no_output "set remote memory-write-packet-size limit"
+gdb_test "show remote memory-write-packet-size" \
+       "The memory-write-packet-size is 0 \\(default\\). The actual limit will be further reduced dependent on the target\." \
+       "write-packet default limit again"
+
 #
 # Part TWO: Check the download behavior.
 #