Re: [PATCH] Do not clear the value of st_dev in File I/O's stat()

[Julio Guerra <julio at farjump dot io>]

Le 17/05/2018 à 16:36, Pedro Alves a écrit :
> [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?
>
Well, this is a little bit like the other patch I submitted (allowing to
open non regular files): I think that File IO values should just pass
through GDB. So running stat() or fstat() on the console (stdin/stdout)
should simply return the same as host's, ie. equivalent to fstat(0 or 1).

>> 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.
Sorry, I'm not familiar with this yet.

-- 
Julio Guerra