public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Christina Schimpe <christina.schimpe@intel.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config
Date: Thu, 13 Jan 2022 16:21:17 +0100	[thread overview]
Message-ID: <20220113152118.1465255-3-christina.schimpe@intel.com> (raw)
In-Reply-To: <20220113152118.1465255-1-christina.schimpe@intel.com>

This patch adds per-remote target variables for the configuration of
memory read- and write packet size. It is a further change in addition
to commit "gdb: Make global feature array a per-remote target array"
to apply the fixme notes described in commit 5b6d1e4.

The former global variables for that configuration are still available
to allow the command line configuration for all future remote
connections.  Similar to the command line configuration of the per-
remote target feature array, the commands

- set remotewritesize (deprecated)
- set remote memory-read-packet-size
- set remote memory-write-packet-size

will configure the current target (if available) and future remote
connections. The show command will display the global configuration
and the packet size of the current remote target, if available.

It is required to adapt the test gdb.base/remote.exp which is failing
for --target_board=native-extended-gdbserver.  With that board GDB
connects to gdbserver at gdb start time.  Due to this patch two loggings
"The target may not be able to.." are shown if the command 'set remote
memory-write-packet-size fixed' is executed while a target is connected
for the current inferior.  To fix this, the clean_restart command is
moved to a later time point of the test.  It is sufficient to be
connected to the server when "runto_main" is executed.  Now the
connection time is similar to a testrun with
--target_board=native-gdbserver.

To allow the user to distinguish between the packet-size configuration
for future connections and for the currently selected target, the
logging of the command 'set remote memory-write-packet-size fixed' is
adapted.
---
 gdb/remote.c                      | 94 +++++++++++++++++++------------
 gdb/testsuite/gdb.base/remote.exp |  7 ++-
 2 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 10f9226827..a270cb688c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -563,8 +563,31 @@ struct packet_config
     enum packet_support support;
   };
 
-/* This global array contains the default configuration for every new
-   per-remote target array.  */
+/* User configurable variables for the number of characters in a
+   memory read/write packet.  MIN (rsa->remote_packet_size,
+   rsa->sizeof_g_packet) is the default.  Some targets need smaller
+   values (fifo overruns, et.al.) and some users need larger values
+   (speed up transfers).  The variables ``preferred_*'' (the user
+   request), ``current_*'' (what was actually set) and ``forced_*''
+   (Positive - a soft limit, negative - a hard limit).  */
+
+struct memory_packet_config
+{
+  const char *name;
+  long size;
+  int fixed_p;
+};
+
+/* These global variables contain the default configuration for every new
+   remote_feature object.  */
+static memory_packet_config memory_read_packet_config =
+{
+  "memory-read-packet-size",
+};
+static memory_packet_config memory_write_packet_config =
+{
+  "memory-write-packet-size",
+};
 static packet_config remote_protocol_packets[PACKET_MAX];
 
 class remote_features
@@ -573,6 +596,9 @@ class remote_features
 
   remote_features ()
   {
+    m_memory_read_packet_config = memory_read_packet_config;
+    m_memory_write_packet_config = memory_write_packet_config;
+
     std::copy (std::begin (remote_protocol_packets),
 	       std::end (remote_protocol_packets),
 	       std::begin (m_protocol_packets));
@@ -591,6 +617,9 @@ class remote_features
 
   void reset_all_packet_configs_support (void);
 
+  memory_packet_config m_memory_read_packet_config;
+  memory_packet_config m_memory_write_packet_config;
+
   packet_config m_protocol_packets[PACKET_MAX];
 };
 
@@ -1842,21 +1871,6 @@ show_remotebreak (struct ui_file *file, int from_tty,
 static unsigned int remote_address_size;
 
 \f
-/* User configurable variables for the number of characters in a
-   memory read/write packet.  MIN (rsa->remote_packet_size,
-   rsa->sizeof_g_packet) is the default.  Some targets need smaller
-   values (fifo overruns, et.al.) and some users need larger values
-   (speed up transfers).  The variables ``preferred_*'' (the user
-   request), ``current_*'' (what was actually set) and ``forced_*''
-   (Positive - a soft limit, negative - a hard limit).  */
-
-struct memory_packet_config
-{
-  const char *name;
-  long size;
-  int fixed_p;
-};
-
 /* 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
@@ -1922,7 +1936,8 @@ remote_target::get_memory_packet_size (struct memory_packet_config *config)
    something really big then do a sanity check.  */
 
 static void
-set_memory_packet_size (const char *args, struct memory_packet_config *config)
+set_memory_packet_size (const char *args, struct memory_packet_config *config,
+			bool target_connected)
 {
   int fixed_p = config->fixed_p;
   long size = config->size;
@@ -1956,9 +1971,16 @@ set_memory_packet_size (const char *args, struct memory_packet_config *config)
 			 ? 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, query_size))
+      if (target_connected
+	  && ! query (_("The target may not be able to correctly handle a %s\n"
+		      "of %ld bytes.  Change the packet size? "),
+		      config->name, query_size))
+	error (_("Packet size not changed."));
+      else if (! target_connected
+	       && ! query (_("Future targets may not be able to correctly "
+			   "handle a %s\nof %ld bytes.  Change the packet size "
+			   "for future remote targets? "),
+			   config->name, query_size))
 	error (_("Packet size not changed."));
     }
   /* Update the config.  */
@@ -1989,16 +2011,15 @@ show_memory_packet_size (struct memory_packet_config *config)
     }
 }
 
-/* FIXME: needs to be per-remote-target.  */
-static struct memory_packet_config memory_write_packet_config =
-{
-  "memory-write-packet-size",
-};
-
 static void
 set_memory_write_packet_size (const char *args, int from_tty)
 {
-  set_memory_packet_size (args, &memory_write_packet_config);
+  remote_target *remote = get_current_remote_target ();
+  if (remote != nullptr)
+    set_memory_packet_size
+      (args, &remote->m_features.m_memory_write_packet_config, true);
+
+  set_memory_packet_size (args, &memory_write_packet_config, false);
 }
 
 static void
@@ -2060,19 +2081,18 @@ show_remote_packet_max_chars (struct ui_file *file, int from_tty,
 long
 remote_target::get_memory_write_packet_size ()
 {
-  return get_memory_packet_size (&memory_write_packet_config);
+  return get_memory_packet_size (&m_features.m_memory_write_packet_config);
 }
 
-/* FIXME: needs to be per-remote-target.  */
-static struct memory_packet_config memory_read_packet_config =
-{
-  "memory-read-packet-size",
-};
-
 static void
 set_memory_read_packet_size (const char *args, int from_tty)
 {
-  set_memory_packet_size (args, &memory_read_packet_config);
+  remote_target *remote = get_current_remote_target ();
+  if (remote != nullptr)
+    set_memory_packet_size
+      (args, &remote->m_features.m_memory_read_packet_config, true);
+
+  set_memory_packet_size (args, &memory_read_packet_config, false);
 }
 
 static void
@@ -2084,7 +2104,7 @@ show_memory_read_packet_size (const char *args, int from_tty)
 long
 remote_target::get_memory_read_packet_size ()
 {
-  long size = get_memory_packet_size (&memory_read_packet_config);
+  long size = get_memory_packet_size (&m_features.m_memory_read_packet_config);
 
   /* FIXME: cagney/1999-11-07: Functions like getpkt() need to get an
      extra buffer size argument before the memory read size can be
diff --git a/gdb/testsuite/gdb.base/remote.exp b/gdb/testsuite/gdb.base/remote.exp
index 1f0869433f..31c5adfd0d 100644
--- a/gdb/testsuite/gdb.base/remote.exp
+++ b/gdb/testsuite/gdb.base/remote.exp
@@ -62,7 +62,7 @@ gdb_test "show remote memory-write-packet-size" \
 
 set test "set remote memory-write-packet-size fixed"
 gdb_test_multiple $test $test {
-    -re "Change the packet size. .y or n. " {
+    -re "Change the packet size for future remote targets. .y or n. " {
 	gdb_test_multiple "y" $test {
 	    -re "$gdb_prompt $" {
 		pass $test
@@ -70,6 +70,7 @@ gdb_test_multiple $test $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"
@@ -129,8 +130,6 @@ proc gdb_load_timed {executable class writesize} {
     pass $test
 }
 
-clean_restart $binfile
-
 # These download tests won't actually download anything on !is_remote
 # target boards, but we run them anyway because it's simpler, and
 # harmless.
@@ -155,6 +154,8 @@ gdb_load_timed $binfile "limit" 0
 # Get the size of random_data table (defaults to 48K).
 set sizeof_random_data [get_sizeof "random_data" 48*1024]
 
+clean_restart $binfile
+
 #
 # Part THREE: Check the upload behavour
 #
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


  parent reply	other threads:[~2022-01-13 15:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-13 15:21 [PATCH 0/3] Apply fixme notes for multi-target support Christina Schimpe
2022-01-13 15:21 ` [PATCH 1/3] gdb: Make global feature array a per-remote target array Christina Schimpe
2022-01-14 19:30   ` Tom Tromey
2022-01-20 16:32     ` Schimpe, Christina
2022-01-18 11:30   ` Andrew Burgess
2022-01-20 17:24     ` Schimpe, Christina
2022-01-13 15:21 ` Christina Schimpe [this message]
2022-01-14 19:33   ` [PATCH 2/3] gdb: Add per-remote target variables for memory read and write config Tom Tromey
2022-01-18 11:39   ` Andrew Burgess
2022-01-24 15:06     ` Schimpe, Christina
2022-02-04 18:03       ` Andrew Burgess
2022-01-13 15:21 ` [PATCH 3/3] gdb: Remove workaround for the vCont packet Christina Schimpe
2022-01-14 19:42   ` Tom Tromey
2022-01-28 13:28     ` Schimpe, Christina

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220113152118.1465255-3-christina.schimpe@intel.com \
    --to=christina.schimpe@intel.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).