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

[Pedro Alves <palves@redhat.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