Re: [PATCH 2/3] Add vFile:fstat: packet to gdbserver

[Pedro Alves <palves@redhat.com>]

On 03/09/2015 11:58 AM, Gary Benson wrote:
> This patch adds a new packet "vFile:fstat:" to gdbserver.
> This can be used by clients to retrieve information about
> open files.

Nit: Strictly speaking, it's a bit backwards to first add
packets to gdbserver that gdb doesn't support.  In general,
I prefer that the gdb patch lands first, documenting the
packets/commands.

> 
> gdb/ChangeLog:
> 
> 	* NEWS: Announce new vFile:fstat packet.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Host I/O Packets): Document vFile:fstat.

These changes above should be in the patch that teaches
GDB about the packets.  Also, I think you're missing
documentation on the qSupported bits.

> 
> gdb/gdbserver/ChangeLog:
> 
> 	* hostio.c (sys/types.h): New include.
> 	(sys/stat.h): Likewise.
> 	(common-remote-fileio.h): Likewise.
> 	(handle_fstat): New function.
> 	(handle_vFile): Handle vFile:fstat packets.
> 	* server.c (handle_query): Report vFile:fstat as supported.

This part looks good.

> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -37407,6 +37407,13 @@ packet is used.  @samp{vFile:write} returns the number of bytes written,
>  which may be shorter than the length of @var{data}, or -1 if an
>  error occurred.
>  
> +@item vFile:fstat: @var{fd}
> +Get information about the open file corresponding to @var{fd}.
> +On success the information is returned as a binary attachment
> +and the return value is the size of this attachment in bytes.
> +If an error occurs the return value is -1.  The format of the
> +returned binary attachment is as described in @ref{struct stat}.

This should mention/reference the qSupported bit too.

Thanks,
Pedro Alves