This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch/rfc] target read/write partial


Hello,

This patch adds target read/write partial methods.

It's almost ready for prime time.  I want to first see through some other target cleanups namely:
+  /* FIXME: cagney/2003-10-15: This code should walk down the target
+     stack looking for a stratum that supports the mechanism.
+     Unfortunatly, there isn't a per-target-stack chain to walk round.
+     Catch-22.  */
and a s/target_ops/target/ transformation.

Note that it includes:
+  /* Transfer up-to LEN bytes of memory starting at OFFSET.  */
+  TARGET_OBJECT_MEMORY
I'm going to need that when implementing a per-target CONVERT_FROM_FUNC_PTR_ADDR.

The attached is what I've ended up committing. I didn't get to the s/target_ops/???/ transformation, but did get to the target stack this code needed.


Now for some code that really uses it ...

Andrew


2003-10-15 Andrew Cagney <cagney@redhat.com>

	* target.h (struct target_ops): Add "to_read_partial" and
	"to_write_partial", delete "to_query".
	(target_read_partial, target_write_partial): Declare.
	(target_read, target_write): Declare.
	(target_query): Delete macro.
	* target.c (target_read_partial): New function.
	(target_write_partial, target_read, target_write): New function.
	(update_current_target): Inherit "to_read_partial" and
	"to_write_partial" instead of "to_query".
	(debug_to_partial_read, debug_to_partial_write): Replace
	"debug_to_query".
	(setup_target_debug): Set "to_read_partial" and "to_write_partial"
	instead of "to_query".
	* remote.c (remote_read_partial): Replace "remote_query".
	(init_remote_ops): Set "to_read_partial" instead of "to_query".
	(init_remote_async_ops): Ditto.
	* kod.c (gdb_kod_query): Make "bufsize" a LONGEST.  Use
	"target_read_partial" instead of "target_query".
	* avr-tdep.c (avr_io_reg_read_command): Make "bufsize" a LONGEST.
	Use "target_read_partial" instead of "target_query".

2003-10-17  Andrew Cagney  <cagney@redhat.com>

	* target.h (struct target_ops): Add "to_read_partial" and
	"to_write_partial", delete "to_query".
	(target_read_partial, target_write_partial): Declare.
	(target_read, target_write): Declare.
	(target_query): Delete macro.
	* target.c (target_read_partial): New function.
	(target_write_partial, target_read, target_write): New function.
	(update_current_target): Delete inheritance of "to_query".  Add
	comments about "to_read_partial" and "to_write_partial".
	(debug_to_partial_read, debug_to_partial_write): New functions.
	(debug_to_query): Delete function.
	(setup_target_debug): Set "to_read_partial" and "to_write_partial"
	instead of "to_query".
	* remote.c (remote_read_partial): Replace "remote_query".
	(init_remote_ops): Set "to_read_partial" instead of "to_query".
	(init_remote_async_ops): Ditto.
	* kod.c (gdb_kod_query): Make "bufsize" a LONGEST.  Use
	"target_read_partial" instead of "target_query".
	* avr-tdep.c (avr_io_reg_read_command): Make "bufsize" a LONGEST.
	Use "target_read_partial" instead of "target_query".

Index: avr-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/avr-tdep.c,v
retrieving revision 1.71
diff -u -r1.71 avr-tdep.c
--- avr-tdep.c	12 Sep 2003 18:40:16 -0000	1.71
+++ avr-tdep.c	17 Oct 2003 17:24:23 -0000
@@ -1359,7 +1359,7 @@
 static void
 avr_io_reg_read_command (char *args, int from_tty)
 {
-  int bufsiz = 0;
+  LONGEST bufsiz = 0;
   char buf[400];
   char query[400];
   char *p;
@@ -1367,22 +1367,23 @@
   unsigned int val;
   int i, j, k, step;
 
-  if (!current_target.to_query)
+  /* Just get the maximum buffer size. */
+  bufsiz = target_read_partial (&current_target, TARGET_OBJECT_AVR,
+				NULL, NULL, 0, 0);
+  if (bufsiz < 0)
     {
       fprintf_unfiltered (gdb_stderr,
 			  "ERR: info io_registers NOT supported by current "
                           "target\n");
       return;
     }
-
-  /* Just get the maximum buffer size. */
-  target_query ((int) 'R', 0, 0, &bufsiz);
   if (bufsiz > sizeof (buf))
     bufsiz = sizeof (buf);
 
   /* Find out how many io registers the target has. */
   strcpy (query, "avr.io_reg");
-  target_query ((int) 'R', query, buf, &bufsiz);
+  target_read_partial (&current_target, TARGET_OBJECT_AVR, query, buf, 0,
+		       bufsiz);
 
   if (strncmp (buf, "", bufsiz) == 0)
     {
@@ -1413,7 +1414,8 @@
         j = nreg - i;           /* last block is less than 8 registers */
 
       snprintf (query, sizeof (query) - 1, "avr.io_reg:%x,%x", i, j);
-      target_query ((int) 'R', query, buf, &bufsiz);
+      target_read_partial (&current_target, TARGET_OBJECT_AVR, query, buf,
+			   0, bufsiz);
 
       p = buf;
       for (k = i; k < (i + j); k++)
Index: kod.c
===================================================================
RCS file: /cvs/src/src/gdb/kod.c,v
retrieving revision 1.8
diff -u -r1.8 kod.c
--- kod.c	18 Mar 2002 02:26:31 -0000	1.8
+++ kod.c	17 Oct 2003 17:24:24 -0000
@@ -87,11 +87,13 @@
 static void
 gdb_kod_query (char *arg, char *result, int *maxsiz)
 {
-  int bufsiz = 0;
+  LONGEST bufsiz = 0;
 
-  /* Check if current target has remote_query capabilities.
-     If not, it does not have kod either.  */
-  if (! current_target.to_query)
+  /* Check if current target has remote_query capabilities.  If not,
+     it does not have kod either.  */
+  bufsiz = target_read_partial (&current_target, TARGET_OBJECT_KOD,
+				NULL, NULL, 0, 0);
+  if (bufsiz < 0)
     {
       strcpy (result,
               "ERR: Kernel Object Display not supported by current target\n");
@@ -99,7 +101,6 @@
     }
 
   /* Just get the maximum buffer size.  */
-  target_query ((int) 'K', 0, 0, &bufsiz);
 
   /* Check if *we* were called just for getting the buffer size.  */
   if (*maxsiz == 0)
@@ -119,7 +120,8 @@
     error ("kod: query argument too long");
 
   /* Send actual request.  */
-  if (target_query ((int) 'K', arg, result, &bufsiz))
+  if (target_read_partial (&current_target, TARGET_OBJECT_KOD,
+			   arg, result, 0, bufsiz) < 0)
     strcpy (result, "ERR: remote query failed");
 }
 
Index: remote.c
===================================================================
RCS file: /cvs/src/src/gdb/remote.c,v
retrieving revision 1.118
diff -u -r1.118 remote.c
--- remote.c	16 Oct 2003 20:51:47 -0000	1.118
+++ remote.c	17 Oct 2003 17:33:32 -0000
@@ -161,8 +161,6 @@
 
 static int stubhex (int ch);
 
-static int remote_query (int /*char */ , char *, char *, int *);
-
 static int hexnumstr (char *, ULONGEST);
 
 static int hexnumnstr (char *, ULONGEST, int);
@@ -5103,41 +5101,47 @@
     printf_filtered ("No loaded section named '%s'.\n", args);
 }
 
-static int
-remote_query (int query_type, char *buf, char *outbuf, int *bufsiz)
+static LONGEST
+remote_read_partial (struct target_ops *ops, enum target_object object,
+		     const char *annex, void *buf,
+		     ULONGEST offset, LONGEST len)
 {
   struct remote_state *rs = get_remote_state ();
   int i;
   char *buf2 = alloca (rs->remote_packet_size);
   char *p2 = &buf2[0];
+  char query_type;
 
-  if (!bufsiz)
-    error ("null pointer to remote bufer size specified");
-
-  /* minimum outbuf size is (rs->remote_packet_size) - if bufsiz is not large enough let 
-     the caller know and return what the minimum size is   */
-  /* Note: a zero bufsiz can be used to query the minimum buffer size */
-  if (*bufsiz < (rs->remote_packet_size))
+  /* Map pre-existing objects onto letters.  DO NOT do this for new
+     objects!!!  Instead specify new query packets.  */
+  switch (object)
     {
-      *bufsiz = (rs->remote_packet_size);
+    case TARGET_OBJECT_KOD:
+      query_type = 'K';
+      break;
+    case TARGET_OBJECT_AVR:
+      query_type = 'R';
+      break;
+    default:
       return -1;
     }
 
+  /* Note: a zero BUF, OFFSET and LEN can be used to query the minimum
+     buffer size.  */
+  if (buf == NULL && offset == 0 && len == 0)
+    return (rs->remote_packet_size);
+  /* Minimum outbuf size is (rs->remote_packet_size) - if bufsiz is
+     not large enough let the caller.  */
+  if (len < (rs->remote_packet_size))
+    return -1;
+  len = rs->remote_packet_size;
+
   /* except for querying the minimum buffer size, target must be open */
   if (!remote_desc)
     error ("remote query is only available after target open");
 
-  /* we only take uppercase letters as query types, at least for now */
-  if ((query_type < 'A') || (query_type > 'Z'))
-    error ("invalid remote query type");
-
-  if (!buf)
-    error ("null remote query specified");
-
-  if (!outbuf)
-    error ("remote query requires a buffer to receive data");
-
-  outbuf[0] = '\0';
+  gdb_assert (annex != NULL);
+  gdb_assert (buf != NULL);
 
   *p2++ = 'q';
   *p2++ = query_type;
@@ -5147,27 +5151,23 @@
      plus one extra in case we are debugging (remote_debug),
      we have PBUFZIZ - 7 left to pack the query string */
   i = 0;
-  while (buf[i] && (i < ((rs->remote_packet_size) - 8)))
+  while (annex[i] && (i < ((rs->remote_packet_size) - 8)))
     {
-      /* bad caller may have sent forbidden characters */
-      if ((!isprint (buf[i])) || (buf[i] == '$') || (buf[i] == '#'))
-	error ("illegal characters in query string");
-
-      *p2++ = buf[i];
+      /* Bad caller may have sent forbidden characters.  */
+      gdb_assert (isprint (annex[i]) && annex[i] != '$' && annex[i] != '#');
+      *p2++ = annex[i];
       i++;
     }
-  *p2 = buf[i];
-
-  if (buf[i])
-    error ("query larger than available buffer");
+  *p2 = '\0';
+  gdb_assert (annex[i] == '\0');
 
   i = putpkt (buf2);
   if (i < 0)
     return i;
 
-  getpkt (outbuf, *bufsiz, 0);
+  getpkt (buf, len, 0);
 
-  return 0;
+  return strlen (buf);
 }
 
 static void
@@ -5445,7 +5445,7 @@
   remote_ops.to_pid_to_str = remote_pid_to_str;
   remote_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_ops.to_stop = remote_stop;
-  remote_ops.to_query = remote_query;
+  remote_ops.to_read_partial = remote_read_partial;
   remote_ops.to_rcmd = remote_rcmd;
   remote_ops.to_stratum = process_stratum;
   remote_ops.to_has_all_memory = 1;
@@ -5965,7 +5965,7 @@
   remote_async_ops.to_pid_to_str = remote_pid_to_str;
   remote_async_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_async_ops.to_stop = remote_stop;
-  remote_async_ops.to_query = remote_query;
+  remote_async_ops.to_read_partial = remote_read_partial;
   remote_async_ops.to_rcmd = remote_rcmd;
   remote_async_ops.to_stratum = process_stratum;
   remote_async_ops.to_has_all_memory = 1;
Index: target.c
===================================================================
RCS file: /cvs/src/src/gdb/target.c,v
retrieving revision 1.57
diff -u -r1.57 target.c
--- target.c	17 Oct 2003 16:09:20 -0000	1.57
+++ target.c	17 Oct 2003 17:33:35 -0000
@@ -159,8 +159,6 @@
 
 static void debug_to_stop (void);
 
-static int debug_to_query (int /*char */ , char *, char *, int *);
-
 /* Pointer to array of target architecture structures; the size of the
    array; the current index into the array; the allocated size of the 
    array.  */
@@ -422,7 +420,8 @@
       INHERIT (to_pid_to_str, t);
       INHERIT (to_extra_thread_info, t);
       INHERIT (to_stop, t);
-      INHERIT (to_query, t);
+      /* Do not inherit to_read_partial.  */
+      /* Do not inherit to_write_partial.  */
       INHERIT (to_rcmd, t);
       INHERIT (to_enable_exception_callback, t);
       INHERIT (to_get_current_exception_event, t);
@@ -1056,6 +1055,90 @@
   return target_xfer_memory_partial (memaddr, buf, len, 1, err);
 }
 
+/* More generic transfers.  */
+
+LONGEST
+target_read_partial (struct target_ops *ops,
+		     enum target_object object,
+		     const char *annex, void *buf,
+		     ULONGEST offset, LONGEST len)
+{
+  struct target_ops *op;
+
+  /* Find the first target stratum that can handle the request.  */
+  for (op = ops;
+       op != NULL && op->to_read_partial == NULL;
+       op = op->beneath)
+    ;
+  if (op == NULL)
+    return -1;
+  
+  /* Now apply the operation at that level.  */
+  return op->to_read_partial (op, object, annex, buf, offset, len);
+}
+
+LONGEST
+target_write_partial (struct target_ops *ops,
+		      enum target_object object,
+		      const char *annex, const void *buf,
+		      ULONGEST offset, LONGEST len)
+{
+  struct target_ops *op;
+
+  /* Find the first target stratum that can handle the request.  */
+  for (op = ops;
+       op != NULL && op->to_write_partial == NULL;
+       op = op->beneath)
+    ;
+  if (op == NULL)
+    return -1;
+  
+  return op->to_write_partial (op, object, annex, buf, offset, len);
+}
+
+/* Wrappers to perform the full transfer.  */
+LONGEST
+target_read (struct target_ops *ops,
+	     enum target_object object,
+	     const char *annex, void *buf,
+	     ULONGEST offset, LONGEST len)
+{
+  LONGEST xfered = 0;
+  while (xfered < len)
+    {
+      LONGEST xfer = target_write_partial (ops, object, annex,
+					   (bfd_byte *) buf + xfered,
+					   offset + xfered, len - xfered);
+      /* Call an observer, notifying them of the xfer progress?  */
+      if (xfer < 0)
+	return xfer;
+      xfered += xfer;
+      QUIT;
+    }
+  return len;
+}
+
+LONGEST
+target_write (struct target_ops *ops,
+	      enum target_object object,
+	      const char *annex, const void *buf,
+	      ULONGEST offset, LONGEST len)
+{
+  LONGEST xfered = 0;
+  while (xfered < len)
+    {
+      LONGEST xfer = target_write_partial (ops, object, annex,
+					   (bfd_byte *) buf + xfered,
+					   offset + xfered, len - xfered);
+      /* Call an observer, notifying them of the xfer progress?  */
+      if (xfer < 0)
+	return xfer;
+      xfered += xfer;
+      QUIT;
+    }
+  return len;
+}
+
 static void
 target_info (char *args, int from_tty)
 {
@@ -2128,14 +2211,42 @@
   fprintf_unfiltered (gdb_stdlog, "target_stop ()\n");
 }
 
-static int
-debug_to_query (int type, char *req, char *resp, int *siz)
+static LONGEST
+debug_to_read_partial (struct target_ops *ops,
+		       enum target_object object,
+		       const char *annex, void *buf,
+		       ULONGEST offset, LONGEST len)
 {
-  int retval;
+  LONGEST retval;
+
+  retval = target_read_partial (&debug_target, object, annex, buf, offset,
+				len);
 
-  retval = debug_target.to_query (type, req, resp, siz);
+  fprintf_unfiltered (gdb_stdlog,
+		      "target_read_partial (%d, %s, 0x%lx,  0x%s, %s) = %s\n",
+		      (int) object, (annex ? annex : "(null)"),
+		      (long) buf, paddr_nz (offset),
+		      paddr_d (len), paddr_d (retval));
 
-  fprintf_unfiltered (gdb_stdlog, "target_query (%c, %s, %s,  %d) = %d\n", type, req, resp, *siz, retval);
+  return retval;
+}
+
+static LONGEST
+debug_to_write_partial (struct target_ops *ops,
+			enum target_object object,
+			const char *annex, const void *buf,
+			ULONGEST offset, LONGEST len)
+{
+  LONGEST retval;
+
+  retval = target_write_partial (&debug_target, object, annex, buf, offset,
+				len);
+
+  fprintf_unfiltered (gdb_stdlog,
+		      "target_write_partial (%d, %s, 0x%lx,  0x%s, %s) = %s\n",
+		      (int) object, (annex ? annex : "(null)"),
+		      (long) buf, paddr_nz (offset),
+		      paddr_d (len), paddr_d (retval));
 
   return retval;
 }
@@ -2237,7 +2348,8 @@
   current_target.to_thread_alive = debug_to_thread_alive;
   current_target.to_find_new_threads = debug_to_find_new_threads;
   current_target.to_stop = debug_to_stop;
-  current_target.to_query = debug_to_query;
+  current_target.to_read_partial = debug_to_read_partial;
+  current_target.to_write_partial = debug_to_write_partial;
   current_target.to_rcmd = debug_to_rcmd;
   current_target.to_enable_exception_callback = debug_to_enable_exception_callback;
   current_target.to_get_current_exception_event = debug_to_get_current_exception_event;
Index: target.h
===================================================================
RCS file: /cvs/src/src/gdb/target.h,v
retrieving revision 1.43
diff -u -r1.43 target.h
--- target.h	17 Oct 2003 13:59:27 -0000	1.43
+++ target.h	17 Oct 2003 17:33:51 -0000
@@ -26,6 +26,7 @@
 struct objfile;
 struct ui_file;
 struct mem_attrib;
+struct target_ops;
 
 /* This include file defines the interface between the main part
    of the debugger, and the part which is target-specific, or
@@ -175,6 +176,77 @@
 /* Given a name (SIGHUP, etc.), return its signal.  */
 enum target_signal target_signal_from_name (char *);
 
+/* Request the transfer of up to LEN 8-bit bytes of the target's
+   OBJECT.  The OFFSET, for a seekable object, specifies the starting
+   point.  The ANNEX can be used to provide additional data-specific
+   information to the target.
+
+   Return the number of bytes actually transfered, zero when no
+   further transfer is possible, and -1 when the transfer is not
+   supported.
+   
+   NOTE: cagney/2003-10-17: The current interface does not support a
+   "retry" mechanism.  Instead it assumes that at least one byte will
+   be transfered on each call.
+
+   NOTE: cagney/2003-10-17: The current interface can lead to
+   fragmented transfers.  Lower target levels should not implement
+   hacks, such as enlarging the transfer, in an attempt to compensate
+   for this.  Instead, the target stack should be extended so that it
+   implements supply/collect methods and a look-aside object cache.
+   With that available, the lowest target can safely and freely "push"
+   data up the stack.
+
+   NOTE: cagney/2003-10-17: Unlike the old query and the memory
+   transfer mechanisms, these methods are explicitly parameterized by
+   the target that it should be applied to.
+
+   NOTE: cagney/2003-10-17: Just like the old query and memory xfer
+   methods, these new methods perform partial transfers.  The only
+   difference is that these new methods thought to include "partial"
+   in the name.  The old code's failure to do this lead to much
+   confusion and duplication of effort as each target object attempted
+   to locally take responsibility for something it didn't have to
+   worry about.
+
+   NOTE: cagney/2003-10-17: For backward compatibility with the
+   "target_query" method that this replaced, when BUF, OFFSET and LEN
+   are NULL/zero, return the "minimum" buffer size.  See "remote.c"
+   for further information.  */
+
+enum target_object
+{
+  /* Kernel Object Display transfer.  See "kod.c" and "remote.c".  */
+  TARGET_OBJECT_KOD,
+  /* AVR target specific transfer.  See "avr-tdep.c" and "remote.c".  */
+  TARGET_OBJECT_AVR,
+  /* Transfer up-to LEN bytes of memory starting at OFFSET.  */
+  TARGET_OBJECT_MEORY
+  /* Possible future ojbects: TARGET_OJBECT_FILE, TARGET_OBJECT_PROC,
+     TARGET_OBJECT_AUXV, ...  */
+};
+
+extern LONGEST target_read_partial (struct target_ops *ops,
+				    enum target_object object,
+				    const char *annex, void *buf,
+				    ULONGEST offset, LONGEST len);
+
+extern LONGEST target_write_partial (struct target_ops *ops,
+				     enum target_object object,
+				     const char *annex, const void *buf,
+				     ULONGEST offset, LONGEST len);
+
+/* Wrappers to perform the full transfer.  */
+extern LONGEST target_read (struct target_ops *ops,
+			    enum target_object object,
+			    const char *annex, void *buf,
+			    ULONGEST offset, LONGEST len);
+
+extern LONGEST target_write (struct target_ops *ops,
+			     enum target_object object,
+			     const char *annex, const void *buf,
+			     ULONGEST offset, LONGEST len);
+
 
 /* If certain kinds of activity happen, target_wait should perform
    callbacks.  */
@@ -271,7 +343,6 @@
     char *(*to_pid_to_str) (ptid_t);
     char *(*to_extra_thread_info) (struct thread_info *);
     void (*to_stop) (void);
-    int (*to_query) (int /*char */ , char *, char *, int *);
     void (*to_rcmd) (char *command, struct ui_file *output);
     struct symtab_and_line *(*to_enable_exception_callback) (enum
 							     exception_event_kind,
@@ -311,6 +382,16 @@
 					      struct objfile *objfile,
 					      CORE_ADDR offset);
 
+    /* See above.  */
+    LONGEST (*to_read_partial) (struct target_ops *ops,
+				enum target_object object,
+				const char *annex, void *buf, 
+				ULONGEST offset, LONGEST len);
+    LONGEST (*to_write_partial) (struct target_ops *ops,
+				 enum target_object object,
+				 const char *annex, const void *buf,
+				 ULONGEST offset, LONGEST len);
+
     int to_magic;
     /* Need sub-structure for target machine related rather than comm related?
      */
@@ -721,16 +802,6 @@
    used by GUIs to implement a stop button.  */
 
 #define target_stop current_target.to_stop
-
-/* Queries the target side for some information.  The first argument is a
-   letter specifying the type of the query, which is used to determine who
-   should process it.  The second argument is a string that specifies which 
-   information is desired and the third is a buffer that carries back the 
-   response from the target side. The fourth parameter is the size of the
-   output buffer supplied.  */
-
-#define	target_query(query_type, query, resp_buffer, bufffer_size)	\
-     (*current_target.to_query) (query_type, query, resp_buffer, bufffer_size)
 
 /* Send the specified COMMAND to the target's monitor
    (shell,interpreter) for execution.  The result of the query is

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]