This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Do not clear the value of st_dev in File I/O's stat()
- From: Pedro Alves <palves at redhat dot com>
- To: Julio Guerra <julio at farjump dot io>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Cc: Corinna Vinschen <vinschen at redhat dot com>
- Date: Thu, 17 May 2018 15:36:23 +0100
- Subject: Re: [PATCH] Do not clear the value of st_dev in File I/O's stat()
- References: <20180517082631.26855-1-julio@farjump.io> <010201636d368a34-7edcde92-3661-4c9a-94b4-a894b9c8e90a-000000@eu-west-1.amazonses.com>
[Hi Corinna, adding you in case you still happen to remember
any of this and have any input. If not, that's fine.]
On 05/17/2018 09:28 AM, Julio Guerra wrote:
> Do not clear the value of st_dev in the fileio stat structure sent to the
> target. It prevents from being able to check the file type on the target.
> Note that the fileio function fstat `remote_fileio_func_fstat()` doesn't clear
> this field.
Curious. This looked like was written in a way like you'd write if you
had a reason to so I did some archaeology to try to find what it was, see
if the reason is still valid.
I found the original File I/O protocol proposal here:
https://www.sourceware.org/ml/gdb/2002-11/msg00107.html
and in there, in the intro, it said:
~~~~~
The protocol is only used for files on the host file system and
for I/O on the console. Character or block special devices, pipes,
named pipes or sockets or any other communication method on the host
system are not supported by this protocol.
~~~~~
and further below, in "B.3 struct stat", it says:
~~~~~
The values of several fields have a restricted meaning and/or
range of values.
st_dev: 0 file
1 console
~~~~~
And the current manual also says:
@item st_dev
A value of 0 represents a file, 1 the console.
And it looks like early on, in:
https://sourceware.org/ml/gdb-patches/2003-03/msg00221.html
we had:
+static void
+remote_fileio_to_fio_stat (struct stat *st, struct fio_stat *fst)
+{
+ /* `st_dev' is set in the calling function */
+ remote_fileio_to_fio_uint ((long) st->st_ino, fst->fst_ino);
Note the comment.
I can't find the comment in the current sources, but it was there
up until:
commit 791c00567a7ccbae3d71e3b63ac43c0b555079dc
Author: Gary Benson <gbenson@redhat.com>
AuthorDate: Wed Mar 11 17:53:57 2015 +0000
Move remote_fileio_to_fio_stat to gdb/common
and note that remote_fileio_func_fstat still does:
if (fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT)
{
host_to_fileio_uint (1, fst.fst_dev);
^^^
"fstat" was only added later on, along the work that Gary did
around that commit above, for this series:
https://sourceware.org/ml/gdb-patches/2015-03/msg00286.html
... which introduced "fstat" in the protocol, and in that case,
the 0/1 st_dev issue was either missed or ignored, so
fstat is not following the documented values.
Now I'm not sure what to do. If we let the real st_dev
through, what shall we fill it in, in the "console" files
case?
>
> 2018-05-16 Julio Guerra <julio@farjump.io>
>
> * remote-fileio.c: do not clear the value of st_dev in File I/O's stat().
Nit, you'd write:
gdb/ChangeLog:
2018-05-16 Julio Guerra <julio@farjump.io>
* remote-fileio.c (remote_fileio_func_stat): Do not clear the
value of st_dev.
Thanks,
Pedro Alves