[v3] Allow using special files with File I/O functions

  Le 29/06/2018 à 15:42, Pedro Alves a écrit :

 


 
 


 

On 06/28/2018 08:27 PM, Julio Guerra wrote:


 

- Remove the restriction to regular files only and add support for special file
types in the File IO stat structure.
- Define a few more macro definitions of file types such as FIFOs, etc.

The major goal is being able to write advanced embedded testing functions, like:
- using a FIFO between the embedded program and the host, instead of being
  restricted only to the GDB console.
- mocking features based on host's by opening some /dev special files.



 

This needs a GDB manual change, and a NEWS entry.

 


 
 


 

Ok.

 


 
 


 




 

2018-06-28  Julio Guerra  <julio@farjump.io>

        * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat):
        Allow using File I/O functions open(), stat() and fstat() on special
        files.
        * ../include/gdb/fileio.h: Add macro definitions for special files,
        both for fst_dev and fst_mode fields of struct fst_stat.
        * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new
        special file types in fst_mode's definition.
        (host_to_fileio_stat): Define fst_dev using the new macro definitions
        and according to the file's type.


 

Note that include/ has its own ChangeLog file.

 


 
 


 

Ok.

 


 
 


 




 

 2018-06-27  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* gdb-gdb.py.in (StructMainTypePrettyPrinter) <to_string>: Don't
 

EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR,
 or includes O_CREAT without O_DIRECTORY.

 

I wait for your comments until I resubmit a v4.

 

Thank you,
  

Hi,

Your message came our hard to read:

 https://sourceware.org/ml/gdb-patches/2018-06/msg00715.html

Please make sure your client is set up to quote replies.

On 06/29/2018 03:01 PM, Julio Guerra wrote:

> I assumed a system >= POSIX.1-2001 here. man 7 inode says:
> 
>  
> 
> The S_IF* constants are present in POSIX.1-2001 and later.
>  […]
>  The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but
>  both are present in POSIX.1-2001
> 
> 

POSIX specification != actual implementations.  Also, Windows != POSIX,
for example.  See the gnulib page I pointed at.  Also, looking through
history with "git blame", etc. may find something.

> Yes, because of man 2 open:
> 
>  
> 
> EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR,
>  or includes O_CREAT without O_DIRECTORY.

I assume you're on Linux, so "man 2" is the Linux Programmer's manual.
But GDB works in other hosts too.  So it may be the code was there for
some other host.  I mean, why did someone write that in the first place?
Again, sounds like some code archaeology is in order.

I forgot to say in the previous email, but it would be really nice if we
could add some coverage for these change to the testsuite.  I've asked
before but I don't remember the answer -- Would it be possible to portably
update e.g. gdb.base/fileio.exp to cover at least one kind
of FILEIO_STDEV_SPECIAL file?

Thanks,
Pedro Alves
  

Le 29/06/2018 à 16:28, Pedro Alves a écrit :
> Hi,

>

> Your message came our hard to read:

>

>  https://sourceware.org/ml/gdb-patches/2018-06/msg00715.html

>

> Please make sure your client is set up to quote replies.

>

> On 06/29/2018 03:01 PM, Julio Guerra wrote:

>

>> I assumed a system >= POSIX.1-2001 here. man 7 inode says:

>>

>>  

>>

>> The S_IF* constants are present in POSIX.1-2001 and later.

>>  […]

>>  The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but

>>  both are present in POSIX.1-2001

>>

>>

> POSIX specification != actual implementations.  Also, Windows != POSIX,

> for example.  See the gnulib page I pointed at.  Also, looking through

> history with "git blame", etc. may find something.


Isn't GDB compiled with mingw? I am no expert in mingw, but I thought it
was a POSIX implementation based on Windows.
Anyway, we can add usual #ifdef guards arounds the cases.
>

>> Yes, because of man 2 open:

>>

>>  

>>

>> EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR,

>>  or includes O_CREAT without O_DIRECTORY.

> I assume you're on Linux, so "man 2" is the Linux Programmer's manual.

> But GDB works in other hosts too.  So it may be the code was there for

> some other host.  I mean, why did someone write that in the first place?

> Again, sounds like some code archaeology is in order.


Yes, but I checked it was POSIX too:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html
>

> I forgot to say in the previous email, but it would be really nice if we

> could add some coverage for these change to the testsuite.  I've asked

> before but I don't remember the answer -- Would it be possible to portably

> update e.g. gdb.base/fileio.exp to cover at least one kind

> of FILEIO_STDEV_SPECIAL file?


Ok, I'll have a look, but I thought there File IOs were not implemented
in gdbserver, so what runs this test? If it is natively run, it won't
cover remote_fileio_func_*.

-- 
Julio Guerra
Co-founder & CTO of Farjump
Mobile: +33 618 644 164
LinkedIn: https://linkedin.com/in/guerrajulio
Slack: farjump.slack.com
  

On 06/29/2018 03:40 PM, Julio Guerra wrote:
> Le 29/06/2018 à 16:28, Pedro Alves a écrit :
>> On 06/29/2018 03:01 PM, Julio Guerra wrote:
>>
>>> I assumed a system >= POSIX.1-2001 here. man 7 inode says:
>>>
>>>  
>>>
>>> The S_IF* constants are present in POSIX.1-2001 and later.
>>>  […]
>>>  The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but
>>>  both are present in POSIX.1-2001
>>>
>>>
>> POSIX specification != actual implementations.  Also, Windows != POSIX,
>> for example.  See the gnulib page I pointed at.  Also, looking through
>> history with "git blame", etc. may find something.
> 
> Isn't GDB compiled with mingw? I am no expert in mingw, but I thought it
> was a POSIX implementation based on Windows.

It's compiled with either MinGW or Cygwin.  Cygwin is POSIX-like, but
mingw is native Windows.

> Anyway, we can add usual #ifdef guards arounds the cases.

That may not be needed, but does not address the S_ISxxx vs S_IFxxx comment.
Again, please take a look at the gnulib header.  ISTM that S_ISxxx is
always available (there's a default implementation that just returns 0),
not sure about S_IFxxx.  But please do take a better look.  I've only
skimmed it.

>>
>>> Yes, because of man 2 open:
>>>
>>>  
>>>
>>> EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR,
>>>  or includes O_CREAT without O_DIRECTORY.
>> I assume you're on Linux, so "man 2" is the Linux Programmer's manual.
>> But GDB works in other hosts too.  So it may be the code was there for
>> some other host.  I mean, why did someone write that in the first place?
>> Again, sounds like some code archaeology is in order.
> 
> Yes, but I checked it was POSIX too:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html

Again, spec vs implementation.  It may not be needed, but I wouldn't be
surprised if that was needed on e.g., Windows either.  Again, archaeology
may help here.

>>
>> I forgot to say in the previous email, but it would be really nice if we
>> could add some coverage for these change to the testsuite.  I've asked
>> before but I don't remember the answer -- Would it be possible to portably
>> update e.g. gdb.base/fileio.exp to cover at least one kind
>> of FILEIO_STDEV_SPECIAL file?
> 
> Ok, I'll have a look, but I thought there File IOs were not implemented
> in gdbserver, so what runs this test? If it is natively run, it won't
> cover remote_fileio_func_*.

You can run the testsuite against other remote stubs, not just gdbserver.
Ideally, you'd set up the testsuite to against your stub.  Did you try
that?  I'd recommend that regardless.

Otherwise, even if we only test it when natively run, other folks that have
embedded stubs that support fileio will end up exercising the test.

Thanks,
Pedro Alves
  

Pedro,

> You can run the testsuite against other remote stubs, not just gdbserver.
> Ideally, you'd set up the testsuite to against your stub.  Did you try
> that?  I'd recommend that regardless.
>
> Otherwise, even if we only test it when natively run, other folks that have
> embedded stubs that support fileio will end up exercising the test.

I have almost finished writing the tests, but I am struggling on how to
create some special files:
- Creating block or character devices require mknod with root privileges.
- Creating a unix socket is not supported by the TCL library, and
require extra tools (netcat, socat...) to create one from the command line.

Here is what I use for now for the other types:
- Link: TCL function `file link`
- Regular: TCL function `open`
- FIFO pipe command line: `mknod myfifo p` or `mkfifo myfifo` which
doesn't exist on Windows.

What do you suggest? Should I avoid every non portable test cases, which
limits the tests only to links, regular files and directories?
  

On 07/04/2018 10:44 AM, Julio Guerra wrote:

>> You can run the testsuite against other remote stubs, not just gdbserver.
>> Ideally, you'd set up the testsuite to against your stub.  Did you try
>> that?  I'd recommend that regardless.
>>
>> Otherwise, even if we only test it when natively run, other folks that have
>> embedded stubs that support fileio will end up exercising the test.
> 
> I have almost finished writing the tests, but I am struggling on how to
> create some special files:
> - Creating block or character devices require mknod with root privileges.
> - Creating a unix socket is not supported by the TCL library, and
> require extra tools (netcat, socat...) to create one from the command line.
> 
> Here is what I use for now for the other types:
> - Link: TCL function `file link`
> - Regular: TCL function `open`
> - FIFO pipe command line: `mknod myfifo p` or `mkfifo myfifo` which
> doesn't exist on Windows.
> 
> What do you suggest? Should I avoid every non portable test cases, which
> limits the tests only to links, regular files and directories?

Yeah, as long as we test at least one "special" file, I think it's OK.

Thanks,
Pedro Alves