From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92940 invoked by alias); 18 May 2018 15:53:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 92929 invoked by uid 89); 18 May 2018 15:53:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Alves, alves, Benson, benson X-HELO: a2-130.smtp-out.eu-west-1.amazonses.com Received: from a2-130.smtp-out.eu-west-1.amazonses.com (HELO a2-130.smtp-out.eu-west-1.amazonses.com) (54.240.2.130) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 18 May 2018 15:53:04 +0000 Subject: Re: [PATCH] Do not clear the value of st_dev in File I/O's stat() From: =?UTF-8?Q?Julio_Guerra?= To: =?UTF-8?Q?Pedro_Alves?= , =?UTF-8?Q?gdb-patches=40s?= =?UTF-8?Q?ourceware=2Eorg?= Cc: =?UTF-8?Q?Corinna_Vinschen?= Date: Fri, 18 May 2018 18:29:00 -0000 Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable In-Reply-To: <9445cc6c-a954-7cdf-d0dc-47a18e596db3@redhat.com> References: <20180517082631.26855-1-julio@farjump.io> <010201636d368a34-7edcde92-3661-4c9a-94b4-a894b9c8e90a-000000@eu-west-1.amazonses.com> <9445cc6c-a954-7cdf-d0dc-47a18e596db3@redhat.com> <496efebf-0f96-4586-de39-0a1857994f04@farjump.io> Message-ID: <0102016373f45297-6fa61071-d39e-4cdd-9908-591b82df782a-000000@eu-west-1.amazonses.com> X-SES-Outgoing: 2018.05.18-54.240.2.130 Feedback-ID: 1.eu-west-1.b24dn6frgCi6dh20skzbuMRr7UL8M6Soir/3ogtEjHQ=:AmazonSES X-IsSubscribed: yes X-SW-Source: 2018-05/txt/msg00405.txt.bz2 Le 17/05/2018 =C3=A0 16:36, Pedro Alves a =C3=A9crit=C2=A0:=0D > [Hi Corinna, adding you in case you still happen to remember=0D > any of this and have any input. If not, that's fine.]=0D >=0D > On 05/17/2018 09:28 AM, Julio Guerra wrote:=0D >> Do not clear the value of st_dev in the fileio stat structure sent to th= e=0D >> target. It prevents from being able to check the file type on the target= .=0D >> Note that the fileio function fstat `remote_fileio_func_fstat()` doesn't= clear=0D >> this field.=0D > Curious. This looked like was written in a way like you'd write if you=0D > had a reason to so I did some archaeology to try to find what it was, see= =0D > if the reason is still valid.=0D >=0D > I found the original File I/O protocol proposal here:=0D >=0D > https://www.sourceware.org/ml/gdb/2002-11/msg00107.html=0D >=0D > and in there, in the intro, it said:=0D >=0D > ~~~~~=0D > The protocol is only used for files on the host file system and=0D > for I/O on the console. Character or block special devices, pipes,=0D > named pipes or sockets or any other communication method on the host=0D > system are not supported by this protocol.=0D > ~~~~~=0D >=0D > and further below, in "B.3 struct stat", it says:=0D >=0D > ~~~~~=0D > The values of several fields have a restricted meaning and/or=0D > range of values.=0D >=0D > st_dev: 0 file=0D > 1 console=0D > ~~~~~=0D >=0D > And the current manual also says:=0D >=0D > @item st_dev=0D > A value of 0 represents a file, 1 the console.=0D >=0D > And it looks like early on, in:=0D >=0D > https://sourceware.org/ml/gdb-patches/2003-03/msg00221.html=0D >=0D > we had:=0D >=0D > +static void=0D > +remote_fileio_to_fio_stat (struct stat *st, struct fio_stat *fst)=0D > +{=0D > + /* `st_dev' is set in the calling function */=0D > + remote_fileio_to_fio_uint ((long) st->st_ino, fst->fst_ino);=0D >=0D > Note the comment. =0D >=0D > I can't find the comment in the current sources, but it was there=0D > up until:=0D >=0D > commit 791c00567a7ccbae3d71e3b63ac43c0b555079dc=0D > Author: Gary Benson =0D > AuthorDate: Wed Mar 11 17:53:57 2015 +0000=0D >=0D > Move remote_fileio_to_fio_stat to gdb/common=0D >=0D >=0D > and note that remote_fileio_func_fstat still does:=0D >=0D > if (fd =3D=3D FIO_FD_CONSOLE_IN || fd =3D=3D FIO_FD_CONSOLE_OUT)=0D > {=0D > host_to_fileio_uint (1, fst.fst_dev);=0D > ^^^=0D >=0D > "fstat" was only added later on, along the work that Gary did=0D > around that commit above, for this series:=0D >=0D > https://sourceware.org/ml/gdb-patches/2015-03/msg00286.html=0D >=0D > ... which introduced "fstat" in the protocol, and in that case,=0D > the 0/1 st_dev issue was either missed or ignored, so=0D > fstat is not following the documented values.=0D >=0D > Now I'm not sure what to do. If we let the real st_dev=0D > through, what shall we fill it in, in the "console" files=0D > case?=0D >=0D Well, this is a little bit like the other patch I submitted (allowing to=0D open non regular files): I think that File IO values should just pass=0D through GDB. So running stat() or fstat() on the console (stdin/stdout)=0D should simply return the same as host's, ie. equivalent to fstat(0 or 1).=0D =0D >> 2018-05-16 Julio Guerra =0D >>=0D >> * remote-fileio.c: do not clear the value of st_dev in File I/O's stat(= ).=0D > Nit, you'd write:=0D >=0D > gdb/ChangeLog:=0D > 2018-05-16 Julio Guerra =0D >=0D > * remote-fileio.c (remote_fileio_func_stat): Do not clear the=0D > value of st_dev.=0D Sorry, I'm not familiar with this yet.=0D =0D -- =0D Julio Guerra=0D =0D