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

Message ID 20180516141830.16859-9-palves@redhat.com
State New
Headers show
Series
  • remote: More multi-target preparation
Related show

Commit Message

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(-)

-- 
2.14.3

Comments

Simon Marchi May 18, 2018, 9:26 p.m. | #1
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
Pedro Alves May 21, 2018, 7:47 p.m. | #2
On 05/18/2018 10:26 PM, Simon Marchi wrote:
> 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.


Yeah, I think the command itself it pretty obscure/weird.

"0" means "default packet size".  I.e., whatever packet size
the remote can handle.  And if you do
"set remote memory-write-packet fixed", then GDB picks
an arbitrary default...
I've spent a few hours today staring at this, trying to
come up with something reasonable-ish, and this is what
I came up with.  I even caught a bug.  I don't think it's worth
it to polish this much more, as this is a pretty obscure
command that most probably nobody uses nowadays.

WDYT?

------------------
From dcca1d189d4e3bdd21af369fa8dc5b450d3effad Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>

Date: Mon, 21 May 2018 19:46:43 +0100
Subject: [PATCH] Handle "show remote memory-write-packet-size" when not
 connected

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.

I think 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:
yyyy-mm-dd  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:
yyyy-mm-dd  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/remote.c                      | 60 ++++++++++++++++++++++++++-------------
 gdb/testsuite/gdb.base/remote.exp | 30 ++++++++++++++++++--
 2 files changed, 67 insertions(+), 23 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 59880a93a8..72254dba31 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -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 =
diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp
index 26361af9a5..ba34441af2 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 \\(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.
 #
-- 
2.14.3
Simon Marchi May 22, 2018, 2:53 a.m. | #3
On 2018-05-21 03:47 PM, Pedro Alves wrote:
> Yeah, I think the command itself it pretty obscure/weird.

> 

> "0" means "default packet size".  I.e., whatever packet size

> the remote can handle.  And if you do

> "set remote memory-write-packet fixed", then GDB picks

> an arbitrary default...

> I've spent a few hours today staring at this, trying to

> come up with something reasonable-ish, and this is what

> I came up with.  I even caught a bug.  I don't think it's worth

> it to polish this much more, as this is a pretty obscure

> command that most probably nobody uses nowadays.

> 

> WDYT?


This is completely reasonnable, thanks.

Simon
Sergio Durigan Junior May 22, 2018, 9:43 p.m. | #4
On Monday, May 21 2018, Pedro Alves wrote:

> On 05/18/2018 10:26 PM, Simon Marchi wrote:

>> 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.

>

> Yeah, I think the command itself it pretty obscure/weird.

>

> "0" means "default packet size".  I.e., whatever packet size

> the remote can handle.  And if you do

> "set remote memory-write-packet fixed", then GDB picks

> an arbitrary default...

> I've spent a few hours today staring at this, trying to

> come up with something reasonable-ish, and this is what

> I came up with.  I even caught a bug.  I don't think it's worth

> it to polish this much more, as this is a pretty obscure

> command that most probably nobody uses nowadays.

>

> WDYT?


I'm seeing a few failures on Debian-s390x-native-extended-gdbserver-m64:

  PASS -> FAIL: gdb.base/remote.exp: write-packet default
  PASS -> FAIL: gdb.base/remote.exp: set write-packet - small
  PASS -> FAIL: gdb.base/remote.exp: set write-packet - very-small
  new FAIL: gdb.base/remote.exp: write-packet default again
  new FAIL: gdb.base/remote.exp: write-packet default limit again
  PASS -> FAIL: gdb.reverse/s390-mvcle.exp: check MVCLE record 2
  PASS -> FAIL: gdb.reverse/s390-mvcle.exp: check MVCLE record 3
  PASS -> KFAIL: gdb.threads/process-dies-while-handling-bp.exp: non_stop=off: cond_bp_target=0: inferior 1 exited

gdb.log shows:

  ...
  show remote memory-write-packet-size
  The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes.
  (gdb) FAIL: gdb.base/remote.exp: write-packet default
  set remote memory-write-packet-size
  Argument required (integer, `fixed' or `limited').
  (gdb) PASS: gdb.base/remote.exp: set write-packet - NULL
  set remote memory-write-packet-size 20
  (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size 20
  show remote memory-write-packet-size
  The memory-write-packet-size is 20. Packets are limited to 20 bytes.
  (gdb) FAIL: gdb.base/remote.exp: set write-packet - small
  set remote memory-write-packet-size 1
  (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size 1
  show remote memory-write-packet-size
  The memory-write-packet-size is 1. Packets are limited to 20 bytes.
  (gdb) FAIL: gdb.base/remote.exp: set write-packet - very-small
  set remote memory-write-packet-size 0
  (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size 0
  show remote memory-write-packet-size
  The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes.
  (gdb) FAIL: gdb.base/remote.exp: write-packet default again
  set remote memory-write-packet-size fixed
  The target may not be able to correctly handle a memory-write-packet-size
  of 16384 bytes. Change the packet size? (y or n) y
  (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size fixed
  show remote memory-write-packet-size
  The memory-write-packet-size is 0 (default). Packets are fixed at 16384 bytes.
  (gdb) PASS: gdb.base/remote.exp: write-packet default fixed
  set remote memory-write-packet-size limit
  (gdb) PASS: gdb.base/remote.exp: set remote memory-write-packet-size limit
  show remote memory-write-packet-size
  The memory-write-packet-size is 0 (default). Packets are limited to 16383 bytes.
  (gdb) FAIL: gdb.base/remote.exp: write-packet default limit again
  set remote memory-write-packet-size 398
  (gdb) PASS: gdb.base/remote.exp: timed download `remote' - limit, 398 - set packet size
  set remote memory-write-packet-size limit
  ...

This is on a try build; unfortunately our BuildBot has a very big queue
right now and the "official" commit hasn't been tested yet.

Please let me know if you need more info.  I haven't actually tested to
see if this was the exact commit that caused the regression, but it
seems to be the one.

> ------------------

> From dcca1d189d4e3bdd21af369fa8dc5b450d3effad Mon Sep 17 00:00:00 2001

> From: Pedro Alves <palves@redhat.com>

> Date: Mon, 21 May 2018 19:46:43 +0100

> Subject: [PATCH] Handle "show remote memory-write-packet-size" when not

>  connected

>

> 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.

>

> I think 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:

> yyyy-mm-dd  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:

> yyyy-mm-dd  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/remote.c                      | 60 ++++++++++++++++++++++++++-------------

>  gdb/testsuite/gdb.base/remote.exp | 30 ++++++++++++++++++--

>  2 files changed, 67 insertions(+), 23 deletions(-)

>

> diff --git a/gdb/remote.c b/gdb/remote.c

> index 59880a93a8..72254dba31 100644

> --- a/gdb/remote.c

> +++ b/gdb/remote.c

> @@ -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 =

> diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp

> index 26361af9a5..ba34441af2 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 \\(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.

>  #

> -- 

> 2.14.3


-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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"
 
 #