Re: [rfc] Correct semantics of target_read_partial, add target_read_whole

[Daniel Jacobowitz <drow at false dot org>]

Here is an improved version of my previous patch.  According to
suggestions I received, I've renamed target_read_whole to the hopefully
clearer target_read_object_alloc (since it reads an entire object, and
allocates memory for it).  I've eliminated the extra use of
target_read; the code to use target_read_partial directly is slightly
more complicated, but more efficient and overall more sensible.  And:
the _partial methods are no longer exported!  Roughly no one ever wants
to use them; use target_read or target_read_object_alloc instead.

Any further comments on this patch?

-- 
Daniel Jacobowitz
CodeSourcery

2006-07-05  Daniel Jacobowitz  <dan@codesourcery.com>

	* target.c (target_read): Stop if target_read_partial returns 0
	when some bytes have already been read.
	(target_write): Likewise for target_write_partial.
	(target_read_partial, target_write_partial): Make static.
	(target_read_object_alloc): New.
	* target.h: Doc fixes.
	(target_read_partial, target_write_partial): Delete prototypes.
	(target_read_object_alloc): New prototype.

	* auxv.c (target_auxv_read): Delete.
	(target_auxv_search, fprint_target_auxv): Use target_read_object_alloc.
	* auxv.h (target_auxv_read): Delete prototype.
	* avr-tdep.c (avr_io_reg_read_command): Use target_read_object_alloc.
	* ia64-tdep.c (getunwind_table, get_kernel_table): Likewise.
	* linux-nat.c (linux_nat_make_corefile_notes): Likewise.
	* procfs.c (procfs_make_note_section): Likewise.
	* remote.c (remote_xfer_partial): Don't loop here.
	* sparc-tdep.c (sparc_fetch_wcookie): Use target_read.

Index: src/gdb/auxv.c
===================================================================
--- src.orig/gdb/auxv.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/auxv.c	2006-07-05 12:14:34.000000000 -0400
@@ -1,6 +1,6 @@
 /* Auxiliary vector support for GDB, the GNU debugger.
 
-   Copyright (C) 2004 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2005, 2006 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -76,43 +76,6 @@ procfs_xfer_auxv (struct target_ops *ops
   return n;
 }
 
-/* Read all the auxv data into a contiguous xmalloc'd buffer,
-   stored in *DATA.  Return the size in bytes of this data.
-   If zero, there is no data and *DATA is null.
-   if < 0, there was an error and *DATA is null.  */
-LONGEST
-target_auxv_read (struct target_ops *ops, gdb_byte **data)
-{
-  size_t auxv_alloc = 512, auxv_pos = 0;
-  gdb_byte *auxv = xmalloc (auxv_alloc);
-  int n;
-
-  while (1)
-    {
-      n = target_read_partial (ops, TARGET_OBJECT_AUXV,
-			       NULL, &auxv[auxv_pos], 0,
-			       auxv_alloc - auxv_pos);
-      if (n <= 0)
-	break;
-      auxv_pos += n;
-      if (auxv_pos < auxv_alloc) /* Read all there was.  */
-	break;
-      gdb_assert (auxv_pos == auxv_alloc);
-      auxv_alloc *= 2;
-      auxv = xrealloc (auxv, auxv_alloc);
-    }
-
-  if (auxv_pos == 0)
-    {
-      xfree (auxv);
-      *data = NULL;
-      return n;
-    }
-
-  *data = auxv;
-  return auxv_pos;
-}
-
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
@@ -148,7 +111,7 @@ target_auxv_search (struct target_ops *o
 {
   CORE_ADDR type, val;
   gdb_byte *data;
-  int n = target_auxv_read (ops, &data);
+  LONGEST n = target_read_object_alloc (ops, TARGET_OBJECT_AUXV, NULL, &data);
   gdb_byte *ptr = data;
   int ents = 0;
 
@@ -184,7 +147,8 @@ fprint_target_auxv (struct ui_file *file
 {
   CORE_ADDR type, val;
   gdb_byte *data;
-  int len = target_auxv_read (ops, &data);
+  LONGEST len = target_read_object_alloc (ops, TARGET_OBJECT_AUXV, NULL,
+					  &data);
   gdb_byte *ptr = data;
   int ents = 0;
 
Index: src/gdb/auxv.h
===================================================================
--- src.orig/gdb/auxv.h	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/auxv.h	2006-07-05 12:14:17.000000000 -0400
@@ -1,6 +1,6 @@
 /* Auxiliary vector support for GDB, the GNU debugger.
 
-   Copyright (C) 2004 Free Software Foundation, Inc.
+   Copyright (C) 2004, 2005, 2006 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -31,12 +31,6 @@
 struct target_ops;		/* Forward declaration.  */
 
 
-/* Read all the auxv data into a contiguous xmalloc'd buffer,
-   stored in *DATA.  Return the size in bytes of this data.
-   If zero, there is no data and *DATA is null.
-   if < 0, there was an error and *DATA is null.  */
-extern LONGEST target_auxv_read (struct target_ops *ops, gdb_byte **data);
-
 /* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
    Return 0 if *READPTR is already at the end of the buffer.
    Return -1 if there is insufficient buffer for a whole entry.
Index: src/gdb/avr-tdep.c
===================================================================
--- src.orig/gdb/avr-tdep.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/avr-tdep.c	2006-07-05 12:13:55.000000000 -0400
@@ -1,7 +1,7 @@
 /* Target-dependent code for Atmel AVR, for GDB.
 
    Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-   2005 Free Software Foundation, Inc.
+   2005, 2006 Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -1323,35 +1323,22 @@ static void
 avr_io_reg_read_command (char *args, int from_tty)
 {
   LONGEST bufsiz = 0;
-  char buf[400];
+  gdb_byte *buf;
   char query[400];
   char *p;
   unsigned int nreg = 0;
   unsigned int val;
   int i, j, k, step;
 
-  /* 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;
-    }
-  if (bufsiz > sizeof (buf))
-    bufsiz = sizeof (buf);
-
   /* Find out how many io registers the target has. */
-  strcpy (query, "avr.io_reg");
-  target_read_partial (&current_target, TARGET_OBJECT_AVR, query, buf, 0,
-		       bufsiz);
+  bufsiz = target_read_object_alloc (&current_target, TARGET_OBJECT_AVR,
+				     "avr.io_reg", &buf);
 
-  if (strncmp (buf, "", bufsiz) == 0)
+  if (bufsiz <= 0)
     {
       fprintf_unfiltered (gdb_stderr,
-			  _("info io_registers NOT supported by target\n"));
+			  _("ERR: info io_registers NOT supported "
+			    "by current target\n"));
       return;
     }
 
@@ -1359,9 +1346,12 @@ avr_io_reg_read_command (char *args, int
     {
       fprintf_unfiltered (gdb_stderr,
 			  _("Error fetching number of io registers\n"));
+      xfree (buf);
       return;
     }
 
+  xfree (buf);
+
   reinitialize_more_filter ();
 
   printf_unfiltered (_("Target has %u io registers:\n\n"), nreg);
@@ -1377,8 +1367,8 @@ avr_io_reg_read_command (char *args, int
         j = nreg - i;           /* last block is less than 8 registers */
 
       snprintf (query, sizeof (query) - 1, "avr.io_reg:%x,%x", i, j);
-      target_read_partial (&current_target, TARGET_OBJECT_AVR, query, buf,
-			   0, bufsiz);
+      bufsiz = target_read_object_alloc (&current_target, TARGET_OBJECT_AVR,
+					 query, &buf);
 
       p = buf;
       for (k = i; k < (i + j); k++)
@@ -1393,6 +1383,8 @@ avr_io_reg_read_command (char *args, int
 		break;
 	    }
 	}
+
+      xfree (buf);
     }
 }
 
Index: src/gdb/ia64-tdep.c
===================================================================
--- src.orig/gdb/ia64-tdep.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/ia64-tdep.c	2006-07-05 12:13:52.000000000 -0400
@@ -1,7 +1,7 @@
 /* Target-dependent code for the IA-64 for GDB, the GNU debugger.
 
-   Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004, 2005 Free Software
-   Foundation, Inc.
+   Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006
+   Free Software Foundation, Inc.
 
    This file is part of GDB.
 
@@ -2458,8 +2458,8 @@ ia64_access_mem (unw_addr_space_t as,
 }
 
 /* Call low-level function to access the kernel unwind table.  */
-static int
-getunwind_table (void *buf, size_t len)
+static LONGEST
+getunwind_table (gdb_byte **buf_p)
 {
   LONGEST x;
 
@@ -2470,10 +2470,11 @@ getunwind_table (void *buf, size_t len)
      we want to preserve fall back to the running kernel's table, then
      we should find a way to override the corefile layer's
      xfer_partial method.  */
-  x = target_read_partial (&current_target, TARGET_OBJECT_UNWIND_TABLE, NULL,
-			   buf, 0, len);
 
-  return (int)x;
+  x = target_read_object_alloc (&current_target, TARGET_OBJECT_UNWIND_TABLE,
+				NULL, buf_p);
+
+  return x;
 }
 
 /* Get the kernel unwind table.  */				 
@@ -2484,14 +2485,15 @@ get_kernel_table (unw_word_t ip, unw_dyn
 
   if (!ktab) 
     {
+      gdb_byte *ktab_buf;
       size_t size;
-      size = getunwind_table (NULL, 0);
-      if ((int)size < 0)
-        return -UNW_ENOINFO;
-      ktab_size = size;
-      ktab = xmalloc (ktab_size);
-      getunwind_table (ktab, ktab_size);
-		          
+
+      ktab_size = getunwind_table (&ktab_buf);
+      if (ktab_size <= 0)
+	return -UNW_ENOINFO;
+      else
+	ktab = (struct ia64_table_entry *) ktab_buf;
+
       for (etab = ktab; etab->start_offset; ++etab)
         etab->info_offset += KERNEL_START;
     }
Index: src/gdb/linux-nat.c
===================================================================
--- src.orig/gdb/linux-nat.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/linux-nat.c	2006-07-05 12:12:58.000000000 -0400
@@ -2697,7 +2697,8 @@ linux_nat_make_corefile_notes (bfd *obfd
       note_data = thread_args.note_data;
     }
 
-  auxv_len = target_auxv_read (&current_target, &auxv);
+  auxv_len = target_read_object_alloc (&current_target, TARGET_OBJECT_AUXV,
+				       NULL, &auxv);
   if (auxv_len > 0)
     {
       note_data = elfcore_write_note (obfd, note_data, note_size,
Index: src/gdb/procfs.c
===================================================================
--- src.orig/gdb/procfs.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/procfs.c	2006-07-05 12:12:50.000000000 -0400
@@ -6130,7 +6130,8 @@ procfs_make_note_section (bfd *obfd, int
       note_data = thread_args.note_data;
     }
 
-  auxv_len = target_auxv_read (&current_target, &auxv);
+  auxv_len = target_read_object_alloc (&current_target, TARGET_OBJECT_AUXV,
+				       NULL, &auxv);
   if (auxv_len > 0)
     {
       note_data = elfcore_write_note (obfd, note_data, note_size,
Index: src/gdb/sparc-tdep.c
===================================================================
--- src.orig/gdb/sparc-tdep.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/sparc-tdep.c	2006-07-05 12:12:01.000000000 -0400
@@ -158,7 +158,7 @@ sparc_fetch_wcookie (void)
   gdb_byte buf[8];
   int len;
 
-  len = target_read_partial (ops, TARGET_OBJECT_WCOOKIE, NULL, buf, 0, 8);
+  len = target_read (ops, TARGET_OBJECT_WCOOKIE, NULL, buf, 0, 8);
   if (len == -1)
     return 0;
 
Index: src/gdb/target.c
===================================================================
--- src.orig/gdb/target.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/target.c	2006-07-05 12:21:09.000000000 -0400
@@ -1341,7 +1341,7 @@ default_xfer_partial (struct target_ops 
    (inbuf, outbuf)", instead of separate read/write methods, make life
    easier.  */
 
-LONGEST
+static LONGEST
 target_read_partial (struct target_ops *ops,
 		     enum target_object object,
 		     const char *annex, gdb_byte *buf,
@@ -1350,7 +1350,7 @@ target_read_partial (struct target_ops *
   return target_xfer_partial (ops, object, annex, buf, NULL, offset, len);
 }
 
-LONGEST
+static LONGEST
 target_write_partial (struct target_ops *ops,
 		      enum target_object object,
 		      const char *annex, const gdb_byte *buf,
@@ -1373,8 +1373,9 @@ target_read (struct target_ops *ops,
 					  (gdb_byte *) buf + xfered,
 					  offset + xfered, len - xfered);
       /* Call an observer, notifying them of the xfer progress?  */
-      if (xfer <= 0)
-	/* Call memory_error?  */
+      if (xfer == 0)
+	return xfered;
+      if (xfer < 0)
 	return -1;
       xfered += xfer;
       QUIT;
@@ -1395,8 +1396,9 @@ target_write (struct target_ops *ops,
 					   (gdb_byte *) buf + xfered,
 					   offset + xfered, len - xfered);
       /* Call an observer, notifying them of the xfer progress?  */
-      if (xfer <= 0)
-	/* Call memory_error?  */
+      if (xfer == 0)
+	return xfered;
+      if (xfer < 0)
 	return -1;
       xfered += xfer;
       QUIT;
@@ -1404,6 +1406,62 @@ target_write (struct target_ops *ops,
   return len;
 }
 
+/* Perform a full target read of unknown size.  */
+
+LONGEST
+target_read_object_alloc (struct target_ops *ops,
+			  enum target_object object,
+			  const char *annex, gdb_byte **buf_p)
+{
+  size_t buf_alloc, buf_pos;
+  gdb_byte *buf;
+  LONGEST n;
+
+  /* This function does not have a length parameter; it reads the
+     entire OBJECT).  Also, it doesn't support objects fetched partly
+     from one target and partly from another (in a different stratum,
+     e.g. a core file and an executable).  Both reasons make it
+     unsuitable for reading memory.  */
+  gdb_assert (object != TARGET_OBJECT_MEMORY);
+
+  /* Start by reading up to 4K at a time.  The target will throttle
+     this number down if necessary.  */
+  buf_alloc = 4096;
+  buf = xmalloc (buf_alloc);
+  buf_pos = 0;
+  while (1)
+    {
+      n = target_read_partial (ops, object, annex, &buf[buf_pos],
+			       buf_pos, buf_alloc - buf_pos);
+      if (n < 0)
+	{
+	  /* An error occurred.  */
+	  xfree (buf);
+	  return -1;
+	}
+      else if (n == 0)
+	{
+	  /* Read all there was.  */
+	  if (buf_pos == 0)
+	    xfree (buf);
+	  else
+	    *buf_p = buf;
+	  return buf_pos;
+	}
+
+      buf_pos += n;
+
+      /* If the buffer is filling up, expand it.  */
+      if (buf_alloc < buf_pos * 2)
+	{
+	  buf_alloc *= 2;
+	  buf = xrealloc (buf, buf_alloc);
+	}
+
+      QUIT;
+    }
+}
+
 /* Memory transfer methods.  */
 
 void
Index: src/gdb/target.h
===================================================================
--- src.orig/gdb/target.h	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/target.h	2006-07-05 12:12:25.000000000 -0400
@@ -180,38 +180,8 @@ extern char *target_signal_to_name (enum
 /* 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.  */
+/* Target objects which can be transfered using target_read,
+   target_write, et cetera.  */
 
 enum target_object
 {
@@ -229,17 +199,17 @@ enum target_object
   /* Possible future objects: TARGET_OBJECT_FILE, TARGET_OBJECT_PROC, ... */
 };
 
-extern LONGEST target_read_partial (struct target_ops *ops,
-				    enum target_object object,
-				    const char *annex, gdb_byte *buf,
-				    ULONGEST offset, LONGEST len);
-
-extern LONGEST target_write_partial (struct target_ops *ops,
-				     enum target_object object,
-				     const char *annex, const gdb_byte *buf,
-				     ULONGEST offset, LONGEST len);
+/* Request that OPS transfer 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, or -1 if the
+   transfer is not supported or otherwise fails.  Return of a positive
+   value less than LEN indicates that no further transfer is possible.
+   Unlike the raw to_xfer_partial interface, callers of these
+   functions do not need to retry partial transfers.  */
 
-/* Wrappers to perform the full transfer.  */
 extern LONGEST target_read (struct target_ops *ops,
 			    enum target_object object,
 			    const char *annex, gdb_byte *buf,
@@ -250,6 +220,22 @@ extern LONGEST target_write (struct targ
 			     const char *annex, const gdb_byte *buf,
 			     ULONGEST offset, LONGEST len);
 
+/* Wrapper to perform a full read of unknown size.  OBJECT/ANNEX will
+   be read using OPS.  The return value will be -1 if the transfer
+   fails or is not supported; 0 if the object is empty; or the length
+   of the object otherwise.  If a positive value is returned, a
+   sufficiently large buffer will be allocated using xmalloc and
+   returned in *BUF_P containing the contents of the object.
+
+   This method should be used for objects sufficiently small to store
+   in a single xmalloc'd buffer, when no fixed bound on the object's
+   size is known in advance.  Don't try to read TARGET_OBJECT_MEMORY
+   through this function.  */
+
+extern LONGEST target_read_object_alloc (struct target_ops *ops,
+					 enum target_object object,
+					 const char *annex, gdb_byte **buf_p);
+
 /* Wrappers to target read/write that perform memory transfers.  They
    throw an error if the memory transfer fails.
 
@@ -409,9 +395,33 @@ struct target_ops
 					      CORE_ADDR load_module_addr,
 					      CORE_ADDR offset);
 
-    /* Perform partial transfers on OBJECT.  See target_read_partial
-       and target_write_partial for details of each variant.  One, and
-       only one, of readbuf or writebuf must be non-NULL.  */
+    /* Request that OPS transfer 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.  Return of a positive value smaller than LEN does
+       not indicate the end of the object, only the end of the
+       transfer; higher level code should continue transferring if
+       desired.  This is handled in target.c.
+
+       The interface does not support a "retry" mechanism.  Instead it
+       assumes that at least one byte will be transfered on each
+       successful 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.
+
+       See target_read and target_write for more information.  One,
+       and only one, of readbuf or writebuf must be non-NULL.  */
+
     LONGEST (*to_xfer_partial) (struct target_ops *ops,
 				enum target_object object, const char *annex,
 				gdb_byte *readbuf, const gdb_byte *writebuf,
Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2006-07-05 12:09:56.000000000 -0400
+++ src/gdb/remote.c	2006-07-05 12:12:01.000000000 -0400
@@ -5147,35 +5147,23 @@ remote_xfer_partial (struct target_ops *
     case TARGET_OBJECT_AUXV:
       if (remote_protocol_packets[PACKET_qPart_auxv].support != PACKET_DISABLE)
 	{
-	  unsigned int total = 0;
-	  while (len > 0)
-	    {
-	      LONGEST n = min ((get_remote_packet_size () - 2) / 2, len);
-	      snprintf (rs->buf, get_remote_packet_size (),
-			"qPart:auxv:read::%s,%s",
-			phex_nz (offset, sizeof offset),
-			phex_nz (n, sizeof n));
-	      i = putpkt (rs->buf);
-	      if (i < 0)
-		return total > 0 ? total : i;
-	      rs->buf[0] = '\0';
-	      getpkt (&rs->buf, &rs->buf_size, 0);
-	      if (packet_ok (rs->buf, &remote_protocol_packets[PACKET_qPart_auxv])
-		  != PACKET_OK)
-		return total > 0 ? total : -1;
-	      if (strcmp (rs->buf, "OK") == 0)
-		break;		/* Got EOF indicator.  */
-	      /* Got some data.  */
-	      i = hex2bin (rs->buf, readbuf, len);
-	      if (i > 0)
-		{
-		  readbuf = (void *) ((char *) readbuf + i);
-		  offset += i;
-		  len -= i;
-		  total += i;
-		}
-	    }
-	  return total;
+	  LONGEST n = min ((get_remote_packet_size () - 2) / 2, len);
+	  snprintf (rs->buf, get_remote_packet_size (),
+		    "qPart:auxv:read::%s,%s",
+		    phex_nz (offset, sizeof offset),
+		    phex_nz (n, sizeof n));
+	  i = putpkt (rs->buf);
+	  if (i < 0)
+	    return i;
+	  rs->buf[0] = '\0';
+	  getpkt (&rs->buf, &rs->buf_size, 0);
+	  if (packet_ok (rs->buf, &remote_protocol_packets[PACKET_qPart_auxv])
+	      != PACKET_OK)
+	    return -1;
+	  if (strcmp (rs->buf, "OK") == 0)
+	    return 0;		/* Got EOF indicator.  */
+	  /* Got some data.  */
+	  return hex2bin (rs->buf, readbuf, len);
 	}
       return -1;