Re: [PATCH v1] gdbserver: widen the stat fields returned by the vFile packets

[Eli Zaretskii <eliz@gnu.org>]

> From: Abhay Kandpal <abhay@linux.ibm.com>
> Cc: aburgess@redhat.com, cel@linux.ibm.com, abhay.k@ibm.com,
>  Abhay Kandpal <abhay@linux.ibm.com>
> Date: Thu,  3 Sep 2026 09:05:55 -0500
> 
> 
> gdb/
> 	* NEWS: Mention the stat64 feature.
> 	* doc/gdb.texinfo (General Query Packets): Document the stat64
> 	feature.
> 	(Host I/O Packets): Mention the wide reply format.
> 	(struct stat64): New node.
> 	* remote-fileio.c (remote_fileio_to_host_ulong8): New function.
> 	(remote_fileio_wide_to_host_stat): New function.
> 	* remote-fileio.h (remote_fileio_wide_to_host_stat): Declare.
> 	* remote.c (PACKET_stat64_feature): New enum value.
> 	(remote_features::stat64_feature): New method.
> 	(remote_protocol_features): Add stat64.
> 	(remote_target::remote_query_supported): Send stat64+.
> 	(fileio_process_fstat_and_lstat_reply): Handle both formats.
> 	(INIT_GDB_FILE): Add the stat64-feature packet command.
> 	* testsuite/gdb.server/fileio-packets.py (decode_stat_reply):
> 	Handle both the narrow and wide reply formats.
> 
> gdbserver/
> 	* hostio.cc (handle_fstat, handle_stat, handle_lstat): Send the
> 	wide struct when the feature is negotiated.
> 	* server.cc (handle_query): Handle and report stat64.
> 	* server.h (client_state::stat64_feature): New member.
> 
> gdbsupport/
> 	* fileio.cc (host_to_fileio_stat_wide): New function.
> 	(narrow_field, fileio_stat_wide_to_narrow): New functions.
> 	* fileio.h (struct fio_stat_wide): New struct.
> 	(bigendian_to_host): New function.
> 	(host_to_fileio_stat_wide, fileio_stat_wide_to_narrow): Declare.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34567
> ---
> This patch is reg tested.
> 
>  gdb/NEWS                                   |  12 +++
>  gdb/doc/gdb.texinfo                        |  49 ++++++++++
>  gdb/remote-fileio.c                        |  35 ++++++-
>  gdb/remote-fileio.h                        |   4 +
>  gdb/remote.c                               |  36 +++++--
>  gdb/testsuite/gdb.server/fileio-packets.py |  59 +++++++-----
>  gdbserver/hostio.cc                        | 106 +++++++++++++++------
>  gdbserver/server.cc                        |   4 +-
>  gdbserver/server.h                         |   5 +
>  gdbsupport/fileio.cc                       |  70 ++++++++++++++
>  gdbsupport/fileio.h                        |  54 ++++++++++-
>  11 files changed, 368 insertions(+), 66 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 222f38e3d52..de8346350ab 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,18 @@
>  
>  *** Changes since GDB 18
>  
> +* Changed remote packets
> +
> +stat64 in qSupported
> +
> +  The new stat64 feature within the qSupported packet allows GDB and
> +  the stub to agree on a wider form of the stat structure returned by
> +  the vFile:fstat, vFile:stat and vFile:lstat packets, in which st_dev,
> +  st_ino, st_nlink, st_rdev and the three timestamps are 8 bytes rather
> +  than 4.  The wider form is used only when both sides report the
> +  feature; otherwise the existing format is used and values which do
> +  not fit are truncated as before.

This is okay, but does it mean that 32-bit timestamps mean that
version will die after 2038?  If so, perhaps this should be mentioned
in the NEWS entry?

> +@item stat64
> +The remote stub supports the wider form of the stat structure returned
> +by the @samp{vFile:fstat}, @samp{vFile:stat} and @samp{vFile:lstat}
> +packets, as described in @ref{struct stat64}.

The "as described in @ref..." paradigm produces nice HTML, but looks
awkward in Info, because it produces "see".  I suggest a more
traditional

  ...by the @samp{vFile:fstat}, @samp{vFile:stat} and @samp{vFile:lstat}
  packets (@pxref{stat64}).

> +If both @value{GDBN} and the stub report the @samp{stat64} feature in
> +the @samp{qSupported} exchange, the wider format described in
> +@ref{struct stat64} is used instead.

Same here.

> +If both @value{GDBN} and the stub report the @samp{stat64} feature in
> +the @samp{qSupported} exchange, the wider format described in
> +@ref{struct stat64} is used instead.

And here.

> +If both @value{GDBN} and the stub report the @samp{stat64} feature in
> +the @samp{qSupported} exchange, the wider format described in
> +@ref{struct stat64} is used instead.

And here.

> +When the @samp{stat64} feature has been negotiated, the buffer returned
> +by the @samp{vFile:fstat}, @samp{vFile:stat} and @samp{vFile:lstat}
> +packets uses the following representation instead:
> +
> +@smallexample
> +struct stat64 @{
> +    unsigned long st_dev;      /* device */
> +    unsigned long st_ino;      /* inode */
> +    mode_t        st_mode;     /* protection */
> +    unsigned long st_nlink;    /* number of hard links */
> +    unsigned int  st_uid;      /* user ID of owner */
> +    unsigned int  st_gid;      /* group ID of owner */
> +    unsigned long st_rdev;     /* device type (if inode device) */
> +    unsigned long st_size;     /* total size, in bytes */
> +    unsigned long st_blksize;  /* blocksize for filesystem I/O */
> +    unsigned long st_blocks;   /* number of blocks allocated */
> +    unsigned long st_atime;    /* time of last access */
> +    unsigned long st_mtime;    /* time of last modification */
> +    unsigned long st_ctime;    /* time of last change */
> +@};
> +@end smallexample

You probably assumed that 'unsigned long' is a 64-bit type.  But that
is not always true, so I suggest to use a more explicit uint64_t type
instead.

> +This structure is of size 92 bytes.  The fields have the same meanings
> +as in @ref{struct stat}, but @code{st_dev}, @code{st_ino},

The "in @ref..." issue again.

> +This representation is only used by the @samp{vFile} packets; the
> +File-I/O protocol always uses @ref{struct stat}.

And here.

Thanks.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>