Patchwork [08/10] Handle "show remote memory-write-packet-size" when not connected

login
register
mail settings
Submitter Pedro Alves
Date May 16, 2018, 2:18 p.m.
Message ID <20180516141830.16859-9-palves@redhat.com>
Download mbox | patch
Permalink /patch/27290/
State New
Headers show

Comments

Pedro Alves - May 16, 2018, 2:18 p.m.
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."

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* remote.c (get_fixed_memory_packet_size): New.
	(get_memory_packet_size): Use it.
	(show_memory_packet_size): Use it.  Don't refer to
	get_memory_packet_size if not connected to a remote target.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/remote.exp: Adjust expected output of "show remote
	memory-write-packet-size".
---
 gdb/remote.c                      | 34 +++++++++++++++++++++++++---------
 gdb/testsuite/gdb.base/remote.exp |  6 +++---
 2 files changed, 28 insertions(+), 12 deletions(-)
Simon Marchi - May 18, 2018, 9:26 p.m.
On 2018-05-16 10:18 AM, Pedro Alves wrote:
> 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."

The result sounds a bit weird:

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

How can the limit be reduced if it is zero?  I don't really know about
this code, is zero a special value that means no limit?  Perhaps it should be
handled differently to make the message clearer.

Simon

Patch

diff --git a/gdb/remote.c b/gdb/remote.c
index 230288b727..7ec8d24a3b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1333,6 +1333,19 @@  struct memory_packet_config
    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;
+  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.  */
 
@@ -1344,12 +1357,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 ();
@@ -1432,10 +1440,18 @@  show_memory_packet_size (struct memory_packet_config *config)
   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 =
diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp
index 26361af9a5..4f0eb10ea4 100644
--- a/gdb/testsuite/gdb.base/remote.exp
+++ b/gdb/testsuite/gdb.base/remote.exp
@@ -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. The actual limit will be further reduced dependent on the target\." \
 	"write-packet default"
 
 gdb_test "set remote memory-write-packet-size" \
@@ -44,12 +44,12 @@  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"
 
 #