Re: [PATCH v2 1/6] target_fileio_read_stralloc: add an optional length parameter

[Andrew Burgess <aburgess@redhat.com>]

Matthieu Longo <matthieu.longo@arm.com> writes:

> Extend target_fileio_read_stralloc with an optional output parameter
> that returns the number of bytes read, excluding the terminating NUL
> byte.
>
> Most callers only need the returned NUL-terminated buffer and can
> ignore the new parameter, but callers that need the number of bytes
> read can now obtain it without recomputing it.
>
> While updating the function, make a couple of minor cleanups by using
> '\0' instead of 0 for character literals and clarifying the function
> documentation.

This looks fine with on minor nit, see below...

>
> Reviewed-By: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> ---
>  gdb/target.c | 11 ++++++++---
>  gdb/target.h | 20 ++++++++++++--------
>  2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/gdb/target.c b/gdb/target.c
> index 5d937f3ae85..ebf0f30092c 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -3547,7 +3547,8 @@ target_fileio_read_alloc (struct inferior *inf, const char *filename,
>  /* See target.h.  */
>  
>  gdb::unique_xmalloc_ptr<char>
> -target_fileio_read_stralloc (struct inferior *inf, const char *filename)
> +target_fileio_read_stralloc (struct inferior *inf, const char *filename,
> +			     LONGEST *len)
>  {
>    gdb_byte *buffer;
>    char *bufstr;
> @@ -3556,17 +3557,21 @@ target_fileio_read_stralloc (struct inferior *inf, const char *filename)
>    transferred = target_fileio_read_alloc_1 (inf, filename, &buffer, 1);
>    bufstr = (char *) buffer;
>  
> +  /* Note: on failure, target_fileio_read_alloc_1 returns -1.  */

There is no needs for a 'Note:' prefix.  By definition, comments are
things about which we should take note.  Just:

  /* On failure target_fileio_read_alloc_1 returns -1.  */

Is fine.

With that fixed:

Approved-By: Andrew Burgess <aburgess@redhat.com>

thanks,
Andrew