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

Pedro Alves
2015-03-10T18:31:25Z

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