Re: [PATCH v2 2/7] Cleanup some docs about memory write

[Pedro Alves <palves@redhat.com>]

On 04/15/2015 08:47 PM, Simon Marchi wrote:
> Some docs seemed outdated. In the case of target_write_memory, the docs
> in target.c and target/target.h diverged a bit, so I tried to find a
> reasonnable in-between version.

"reasonable"

> 
> gdb/ChangeLog:
> 
> 	* corefile.c (write_memory): Update doc.
> 	* gdbcore.h (write_memory): Same.
> 	* target.c (target_write_memory): Same.
> 	* target/target.h (target_write_memory): Same.
> ---
>  gdb/corefile.c      |  4 ++--
>  gdb/gdbcore.h       |  6 ++----
>  gdb/target.c        |  6 +-----
>  gdb/target/target.h | 10 +++++-----
>  4 files changed, 10 insertions(+), 16 deletions(-)
> 
> diff --git a/gdb/corefile.c b/gdb/corefile.c
> index a042e6d..83b0e80 100644
> --- a/gdb/corefile.c
> +++ b/gdb/corefile.c
> @@ -383,8 +383,8 @@ read_memory_typed_address (CORE_ADDR addr, struct type *type)
>    return extract_typed_address (buf, type);
>  }
>  
> -/* Same as target_write_memory, but report an error if can't
> -   write.  */
> +/* See gdbcore.h. */

Double space after period.

> diff --git a/gdb/target.c b/gdb/target.c
> index fcf7090..bd9a0eb 100644
>  int
>  target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr, ssize_t len)

> diff --git a/gdb/target/target.h b/gdb/target/target.h
> index 05ac758..e0b7554 100644
> --- a/gdb/target/target.h
> +++ b/gdb/target/target.h
> @@ -49,12 +49,12 @@ extern int target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr,
>  extern int target_read_uint32 (CORE_ADDR memaddr, uint32_t *result);
>  
>  /* Write LEN bytes from MYADDR to target memory at address MEMADDR.
> -   Return zero for success, nonzero if any error occurs.  This
> +   Return zero for success or TARGET_XFER_E_IO if any error occurs.  This
>     function must be provided by the client.  Implementations of this
> -   function may define and use their own error codes, but functions
> -   in the common, nat and target directories must treat the return
> -   code as opaque.  No guarantee is made about the contents of the
> -   data at MEMADDR if any error occurs.  */
> +   function may define and use their own error codes, but functions in
> +   the common, nat and target directories must treat the return code as
> +   opaque.  No guarantee is made about how much data got written if any
> +   error occurs.  */
>  
>  extern int target_write_memory (CORE_ADDR memaddr, const gdb_byte *myaddr,
>  				ssize_t len);
> 

This one's not right.  TARGET_XFER_E_IO is a GDB-specific thing, while
this declaration is meant for both gdb and gdbserver.  It's what
"This function must be provided by the client" refers to.  It doesn't
make sense to say "TARGET_XFER_E_IO" one error and then explain that
the return code is opaque.

So probably it'd be better to leave this comment:

> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1464,11 +1464,7 @@ target_read_code (CORE_ADDR memaddr, gdb_byte *myaddr, ssize_t len)
>      return TARGET_XFER_E_IO;
>  }
>
> -/* Write LEN bytes from MYADDR to target memory at address MEMADDR.
> -   Returns either 0 for success or TARGET_XFER_E_IO if any
> -   error occurs.  If an error occurs, no guarantee is made about how
> -   much data got written.  Callers that can deal with partial writes
> -   should call target_write.  */
> +/* See target/target.h.  */

... but extend it by starting by saying that this is GDB's implementation
of the function as declared in target/target.h.

Thanks,
Pedro Alves