Re: [PATCH] zero-terminate result of target_read_alloc

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Mon, 24 Jul 2006 18:00:43 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> All right!  Compromise and progress!  Thanks, Mark.
> 
> How about this patch (untested so far)?  My only concern is that it
> doesn't report error/unsupported separately from empty; but for all the
> uses I know of so far, that's not a problem, and we can change it later
> if we need to.

Better get this right from the start, and return xstrdup("") for
"empty".  Also the error message

"... contained unexpected zero bytes"

is not very clear; the first time I thought that would warn about
returning an empty string.  How about changing that to

"... contained unexpected null characters"

?  

The C standard uses the term "null character" for a byte with all bits
set to 0.

> 2006-07-24  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* target.h (target_read_stralloc): New prototype.
> 	* target.c (target_read_alloc_1): Renamed from target_read_alloc.
> 	Take new PADDING argument.
> 	(target_read_alloc): Use it.
> 	(target_read_stralloc): New function.
> 
> Index: target.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.h,v
> retrieving revision 1.85
> diff -u -p -r1.85 target.h
> --- target.h	12 Jul 2006 18:13:45 -0000	1.85
> +++ target.h	24 Jul 2006 21:59:03 -0000
> @@ -236,6 +236,14 @@ extern LONGEST target_read_alloc (struct
>  				  enum target_object object,
>  				  const char *annex, gdb_byte **buf_p);
>  
> +/* Read OBJECT/ANNEX using OPS.  The result is NUL-terminated
> +   and returned as a string.  A warning is issued if the result
> +   contains any embedded NUL bytes.  */
> +
> +extern char *target_read_stralloc (struct target_ops *ops,
> +				   enum target_object object,
> +				   const char *annex);
> +
>  /* Wrappers to target read/write that perform memory transfers.  They
>     throw an error if the memory transfer fails.
>  
> Index: target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.c,v
> retrieving revision 1.121
> diff -u -p -r1.121 target.c
> --- target.c	18 Jul 2006 12:44:48 -0000	1.121
> +++ target.c	24 Jul 2006 21:59:03 -0000
> @@ -1406,22 +1406,15 @@ target_write (struct target_ops *ops,
>    return 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.  */
> -
> -LONGEST
> -target_read_alloc (struct target_ops *ops,
> -		   enum target_object object,
> -		   const char *annex, gdb_byte **buf_p)
> +/* Read OBJECT/ANNEX using OPS.  Store the result in *BUF_P and return
> +   the size of the transferred data.  PADDING additional bytes are
> +   available in *BUF_P.  This is a helper function for
> +   target_read_alloc; see the declaration of that function for more
> +   information.  */
> +
> +static LONGEST
> +target_read_alloc_1 (struct target_ops *ops, enum target_object object,
> +		     const char *annex, gdb_byte **buf_p, int padding)
>  {
>    size_t buf_alloc, buf_pos;
>    gdb_byte *buf;
> @@ -1442,7 +1435,7 @@ target_read_alloc (struct target_ops *op
>    while (1)
>      {
>        n = target_read_partial (ops, object, annex, &buf[buf_pos],
> -			       buf_pos, buf_alloc - buf_pos);
> +			       buf_pos, buf_alloc - buf_pos - padding);
>        if (n < 0)
>  	{
>  	  /* An error occurred.  */
> @@ -1472,6 +1465,41 @@ target_read_alloc (struct target_ops *op
>      }
>  }
>  
> +/* Read OBJECT/ANNEX using OPS.  Store the result in *BUF_P and return
> +   the size of the transferred data.  See the declaration in "target.h"
> +   function for more information about the return value.  */
> +
> +LONGEST
> +target_read_alloc (struct target_ops *ops, enum target_object object,
> +		   const char *annex, gdb_byte **buf_p)
> +{
> +  return target_read_alloc_1 (ops, object, annex, buf_p, 0);
> +}
> +
> +/* Read OBJECT/ANNEX using OPS.  The result is NUL-terminated
> +   and returned as a string.  A warning is issued if the result
> +   contains any embedded NUL bytes.  */
> +
> +char *
> +target_read_stralloc (struct target_ops *ops, enum target_object object,
> +		      const char *annex)
> +{
> +  gdb_byte *buffer;
> +  LONGEST transferred;
> +
> +  transferred = target_read_alloc_1 (ops, object, annex, &buffer, 1);
> +
> +  if (transferred <= 0)
> +    return NULL;
> +
> +  buffer[transferred] = 0;
> +  if (strlen (buffer) < transferred)
> +    warning (_("target object %d, annex %s, contained unexpected zero bytes"),
> +	     (int) object, annex ? annex : "(none)");
> +
> +  return (char *) buffer;
> +}
> +
>  /* Memory transfer methods.  */
>  
>  void
>