Re: [PATCH] zero-terminate result of target_read_alloc

Mark Kettenis
2006-07-26T21:53:39Z

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