Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch/rfc] target read/write partial
@ 2003-10-15 22:16 Andrew Cagney
  2003-10-15 22:32 ` Daniel Jacobowitz
  2003-10-17 18:05 ` Andrew Cagney
  0 siblings, 2 replies; 5+ messages in thread
From: Andrew Cagney @ 2003-10-15 22:16 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

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.

Otherwize, comments?
Andrew

[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 18352 bytes --]

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

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	15 Oct 2003 22:09:13 -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	15 Oct 2003 22:09:13 -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.116
diff -u -r1.116 remote.c
--- remote.c	2 Oct 2003 20:28:30 -0000	1.116
+++ remote.c	15 Oct 2003 22:09:14 -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);
@@ -5107,41 +5105,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;
@@ -5151,27 +5155,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
@@ -5449,7 +5449,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;
@@ -5969,7 +5969,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.55
diff -u -r1.55 target.c
--- target.c	2 Oct 2003 20:28:30 -0000	1.55
+++ target.c	15 Oct 2003 22:09:15 -0000
@@ -162,8 +162,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.  */
@@ -605,7 +603,8 @@
       INHERIT (to_pid_to_str, t);
       INHERIT (to_extra_thread_info, t);
       INHERIT (to_stop, t);
-      INHERIT (to_query, t);
+      INHERIT (to_read_partial, t);
+      INHERIT (to_write_partial, t);
       INHERIT (to_rcmd, t);
       INHERIT (to_enable_exception_callback, t);
       INHERIT (to_get_current_exception_event, t);
@@ -1072,6 +1071,83 @@
   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)
+{
+  /* 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.  */
+  if (ops->to_write_partial != NULL)
+    return ops->to_read_partial (ops, object, annex, buf, offset, len);
+  else
+    return -1;
+}
+
+LONGEST
+target_write_partial (struct target_ops *ops,
+		      enum target_object object,
+		      const char *annex, void *buf,
+		      ULONGEST offset, LONGEST len)
+{
+  /* 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.  */
+  if (ops->to_write_partial != NULL)
+    return ops->to_write_partial (ops, object, annex, buf, offset, len);
+  else
+    return -1;
+}
+
+/* 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, 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)
 {
@@ -2156,14 +2232,40 @@
   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 = debug_target.to_read_partial (ops, 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 = debug_target.to_write_partial (ops, 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;
 }
@@ -2265,7 +2367,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.41
diff -u -r1.41 target.h
--- target.h	17 Jun 2003 20:28:13 -0000	1.41
+++ target.h	15 Oct 2003 22:09:15 -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,61 @@
 /* Given a name (SIGHUP, etc.), return its signal.  */
 enum target_signal target_signal_from_name (char *);
 \f
+/* 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-15: 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-15: 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-15: 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_MEMORY
+  /* 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, 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, void *buf,
+			     ULONGEST offset, LONGEST len);
+\f
 
 /* If certain kinds of activity happen, target_wait should perform
    callbacks.  */
@@ -293,7 +349,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,
@@ -333,6 +388,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?
      */
@@ -755,16 +820,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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch/rfc] target read/write partial
  2003-10-15 22:16 [patch/rfc] target read/write partial Andrew Cagney
@ 2003-10-15 22:32 ` Daniel Jacobowitz
  2003-10-15 23:09   ` Andrew Cagney
  2003-10-17 18:05 ` Andrew Cagney
  1 sibling, 1 reply; 5+ messages in thread
From: Daniel Jacobowitz @ 2003-10-15 22:32 UTC (permalink / raw)
  To: gdb-patches

On Wed, Oct 15, 2003 at 06:16:28PM -0400, Andrew Cagney wrote:
> 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.

Preferably not target - didn't someone suggest gdbtarg?  Or maybe
gdb_target.

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

How is that different from a memory read, which we've already got? I am
guessing that it's because you want to support partial memory
operations (avoid packet size limits), but you never explained your
goal.

> +  /* Map pre-existing objects onto letters.  DO NOT do this for new
> +     objects!!!  Instead specify new query packets.  */

Could that be a little clearer - I had to read the code a couple of
times to figure out what you meant.  I guess you just want to say that
there's no need to use single letters?

> +  /* Minimum outbuf size is (rs->remote_packet_size) - if bufsiz is
> +     not large enough let the caller.  */

Missing a word there I think.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch/rfc] target read/write partial
  2003-10-15 22:32 ` Daniel Jacobowitz
@ 2003-10-15 23:09   ` Andrew Cagney
  2003-10-15 23:12     ` Daniel Jacobowitz
  0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cagney @ 2003-10-15 23:09 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: gdb-patches

> On Wed, Oct 15, 2003 at 06:16:28PM -0400, Andrew Cagney wrote:
> 
>> 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.
> 
> 
> Preferably not target - didn't someone suggest gdbtarg?  Or maybe
> gdb_target.

... you mean someone actually likes gdbtarg :-)

>> 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.
> 
> 
> How is that different from a memory read, which we've already got? I am
> guessing that it's because you want to support partial memory
> operations (avoid packet size limits), but you never explained your
> goal.

I previously wrote:

> In the light of Joel's "upload/download" commands, and lessons (gdb/589) learn't from similar interfaces, and a realization that I need this for memory:
...
> - it takes an explicit target vector
> - there are both read and write variants (instead of query)
> - it, what the heck, takes a LONGEST
> - it makes the fact that the method isn't expected to perform a full transfer explicit

gdb/589 is yet another example of how the existing code always did 
partial xfers, yet no one noticed.  Have a look at how many levels of 
GDB code try to locally solve the partial transfer problem when 
generic_load is called.

The only significant difference is the addition of an explicit target 
vector.  But that's really significant.  I should probably comment out 
TARGET_OBJECT_MEMORY until a target implements it.

>> +  /* Map pre-existing objects onto letters.  DO NOT do this for new
>> +     objects!!!  Instead specify new query packets.  */
> 
> 
> Could that be a little clearer - I had to read the code a couple of
> times to figure out what you meant.  I guess you just want to say that
> there's no need to use single letters?

The code abuses:

	<letter> + <annex>

as a way of generating packets.  The entire qK* and qR* packet maps have 
been kidnapped by this.  The un-approved RedBoot patches did the same 
with qM*.

I'll expand the comment.

>> +  /* Minimum outbuf size is (rs->remote_packet_size) - if bufsiz is
>> +     not large enough let the caller.  */
> 
> 
> Missing a word there I think.

Will fix.

Andrew



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch/rfc] target read/write partial
  2003-10-15 23:09   ` Andrew Cagney
@ 2003-10-15 23:12     ` Daniel Jacobowitz
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Jacobowitz @ 2003-10-15 23:12 UTC (permalink / raw)
  To: gdb-patches

On Wed, Oct 15, 2003 at 07:09:16PM -0400, Andrew Cagney wrote:
> >On Wed, Oct 15, 2003 at 06:16:28PM -0400, Andrew Cagney wrote:
> >
> >>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.
> >
> >
> >Preferably not target - didn't someone suggest gdbtarg?  Or maybe
> >gdb_target.
> 
> ... you mean someone actually likes gdbtarg :-)

Not all that much.  But I really dislike "target".

> >>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.
> >
> >
> >How is that different from a memory read, which we've already got? I am
> >guessing that it's because you want to support partial memory
> >operations (avoid packet size limits), but you never explained your
> >goal.
> 
> I previously wrote:
> 
> >In the light of Joel's "upload/download" commands, and lessons (gdb/589) 
> >learn't from similar interfaces, and a realization that I need this for 
> >memory:
> ...
> >- it takes an explicit target vector
> >- there are both read and write variants (instead of query)
> >- it, what the heck, takes a LONGEST
> >- it makes the fact that the method isn't expected to perform a full 
> >transfer explicit
> 
> gdb/589 is yet another example of how the existing code always did 
> partial xfers, yet no one noticed.  Have a look at how many levels of 
> GDB code try to locally solve the partial transfer problem when 
> generic_load is called.
> 
> The only significant difference is the addition of an explicit target 
> vector.  But that's really significant.  I should probably comment out 
> TARGET_OBJECT_MEMORY until a target implements it.

Yes, please.  Anyway, I see where you're going now.

> >>+  /* Map pre-existing objects onto letters.  DO NOT do this for new
> >>+     objects!!!  Instead specify new query packets.  */
> >
> >
> >Could that be a little clearer - I had to read the code a couple of
> >times to figure out what you meant.  I guess you just want to say that
> >there's no need to use single letters?
> 
> The code abuses:
> 
> 	<letter> + <annex>
> 
> as a way of generating packets.  The entire qK* and qR* packet maps have 
> been kidnapped by this.  The un-approved RedBoot patches did the same 
> with qM*.
> 
> I'll expand the comment.

Do we need a standard terminator here too - colon or semicolon again?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch/rfc] target read/write partial
  2003-10-15 22:16 [patch/rfc] target read/write partial Andrew Cagney
  2003-10-15 22:32 ` Daniel Jacobowitz
@ 2003-10-17 18:05 ` Andrew Cagney
  1 sibling, 0 replies; 5+ messages in thread
From: Andrew Cagney @ 2003-10-17 18:05 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1944 bytes --]

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


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 19268 bytes --]

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 *);
 \f
+/* 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);
+\f
 
 /* 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2003-10-17 18:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-15 22:16 [patch/rfc] target read/write partial Andrew Cagney
2003-10-15 22:32 ` Daniel Jacobowitz
2003-10-15 23:09   ` Andrew Cagney
2003-10-15 23:12     ` Daniel Jacobowitz
2003-10-17 18:05 ` Andrew Cagney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox