Re: [PATCH v3 2/2] gdb: add stat64 and fstat64 to the F-packet file-i/o protocol

[Ulrich Weigand <Ulrich.Weigand@de.ibm.com>]

Abhay Kandpal <abhay@linux.ibm.com> wrote:

>The F-packet calls cannot use qSupported, since the buffer is read
>directly by the target.  But each request names the call it wants, so
>add two new calls, Fstat64 and Ffstat64, which are identical to Fstat
>and Ffstat except that they write struct fio_stat_wide.  Existing
>targets keep using Fstat and Ffstat and see no change.  A GDB which
>does not know the new calls already replies with ENOSYS, so a target
>can try Fstat64 and fall back to Fstat.

This makes sense to me, if support for this is needed.  I'm not
really sure who -if anyone- is using these F-packets; gdbserver
doesn't seem to do so and there's no test in the test suite ...

Andrew, do you have any thoughts on this?


>The console case in the fstat handler stored 1 into fst_dev, but that
>was then overwritten when the struct was packed, so the target always
>saw zero.  Drop the dead store; behaviour is unchanged.

Interesting.  Seems this accidentally changed here:
https://sourceware.org/pipermail/gdb-patches/2015-March/122143.html

which says:
	(remote_fileio_mode_to_target): Moved to common-remote-
fileio.c.
	(remote_fileio_to_fio_mode): Likewise.
	(remote_fileio_to_fio_ulong): Likewise.
	(remote_fileio_to_fio_stat): Likewise.

but in addition to the move, changed:

-static void
-remote_fileio_to_fio_stat (struct stat *st, struct fio_stat *fst)
-{
-  LONGEST blksize;
-
-  /* `st_dev' is set in the calling function.  */
-  remote_fileio_to_fio_uint ((long) st->st_ino, fst->fst_ino);

to:

+void
+remote_fileio_to_fio_stat (struct stat *st, struct fio_stat *fst)
+{
+  LONGEST blksize;
+
+  remote_fileio_to_fio_uint ((long) st->st_dev, fst->fst_dev);
+  remote_fileio_to_fio_uint ((long) st->st_ino, fst->fst_ino);


So up until 2015 fst_dev was indeed 1 for the console and
0 for everything else, and since then it's been 0 always.
Given that long time, it may be fine to keep it at 0 now ...

On the other hand, the documentation still says:

@item st_dev
A value of 0 represents a file, 1 the console.

So we can either fix the code again, or else update the
documentation to match the behavior over the last 10 years.
Thoughts?

Bye,
Ulrich