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

["Julio Guerra" <julio@farjump.io>]

>>>> Signed-off-by: Julio Guerra <julio@farjump.io>
>>> I'm not sure whether I asked this before, but, just in case,
>>> do you have a copyright assignment on file with the FSF?
>>> I looked for one now and couldn't find it.
>>>
>> No, I don't.
> See:
>
>  https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
>
> The request-assign.future one is most common one.  Please follow the
> instructions at the top of the file.

Done.

>
>>>> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
>>>> index 313da642ea..168590245e 100644
>>>> --- a/gdb/remote-fileio.c
>>>> +++ b/gdb/remote-fileio.c
>>>> @@ -885,16 +885,9 @@ remote_fileio_func_stat (remote_target *remote, char *buf)
>>>>        remote_fileio_return_errno (remote, -1);
>>>>        return;
>>>>      }
>>>> -  /* Only operate on regular files and directories.  */
>>>> -  if (!ret && !S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode))
>>>> -    {
>>>> -      remote_fileio_reply (remote, -1, FILEIO_EACCES);
>>>> -      return;
>>>> -    }
>>> What happens if we stat/open some kind of unsupported file type?
>>> Do we end up with st_mode == 0 and report success anyway, or is
>>> something else catching it and returning FILEIO_EACCES or some such?
>>>
>> Yes, bits SFMT of st_mode end up with everything 0 and it doesn't fail.
>> It's like not knowing what kind of file it exactly is, but still get
>> other values.
> Hmm, OK.  I mildly worry whether that that might cause trouble.
> I wonder what other filesystem network protocols do here.  Like,
> e.g.,  nfs, sshfs, etc.
>

The 3 options I see:
1 - No restrinction: this one.
2 - Warning: test SFMT bits and when "unsupported", set File IO SFMT
bits to some specific FILEIO_S_UNKNOWN value.
3 - Restrict: strictly restrict to those SFMT types I added.

I would go for 1 or 2, to avoid another similar restriction like there
was in open before this patch.

>>>>    /* Nonexistant file */
>>>>    errno = 0;
>>>>    ret = stat (NONEXISTANT, &st);
>>>> -  printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>>>> -  	  strerrno (errno));
>>>> +  if (!ret)
>>>> +    printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>>>> +	    strerrno (errno));
>>>> +  else
>>>> +    printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
>>> Do we want to print errno in the !ret case?  That indicates the
>>> stat call succeeded (even though we expect it to fail).
>>> Might be that helps simplify the .exp, I haven't checked.
>>>
>>> But at least the strerrno call should be on the else
>>> branch, as that's the branch where errno is meaninful, and
>>> the .exp expects it:
>>>
>>>   FAIL: gdb.base/fileio.exp: Stat a nonexistant file returns ENOENT
>>>
>>> Did this pass against your stub?
>> Yes, here are logs of GDB's execution on this case:
>>
>>> Continuing.
>>> stat 4: ret = -1, errno = 2 ENOENT
> I don't see how that could have been logs for the patch you sent,
> because if ret really was -1, then that would have said "stat 1",
> and would not have printed ENOENT.  Maybe you tested a
> different patch?

I guess I didn't  run the tests again after this last 'if ... else'
modification, sorry...


>>>> diff --git a/gdb/testsuite/gdb.base/fileio.exp b/gdb/testsuite/gdb.base/fileio.exp
>>>> index bc409c26aa..234f304ac7 100644
>>>> --- a/gdb/testsuite/gdb.base/fileio.exp
>>>> +++ b/gdb/testsuite/gdb.base/fileio.exp
>>>> @@ -31,7 +31,7 @@ if {[is_remote host]} {
>>>>  
>>>>  if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>>>>  	   executable \
>>>> -	   [list debug "additional_flags=-DOUTDIR=\"$outdir/\""]] != "" } {
>>>> +					 [list debug "additional_flags=-DOUTDIR=\"$outdir/\" [target_info fileio,cflags]" "ldflags=[target_info fileio,ldflags]"]] != "" } {
>>> I couldn't tell what's this change for?  Why did you need it?
>> I couldn't find any other way of adding some CFLAGS and LDFLAGS to the
>> call to the cross-compiler to link against the libc using File IOs, to
>> add target-specific compilation flags, etc. For example, in my case:
>>
>>> set_board_info fileio,cflags "--specs=$sdk/Alpha.specs
>> -mfloat-abi=hard -mfpu=vfp -march=armv6zk -mtune=arm1176jzf-s"
>>> set_board_info fileio,ldflags "-Wl,-T$sdk/link.ld"
> Is this something really specific to this testcase?  Don't you
> need to do the same for all other testcases?

Yes, every other tests involving cross-compilation and the C library
need the same.

> Did you try CC_FOR_TARGET/LD_FOR_TARGET?

I just tried and couldn't make it work. Common use cases are indeed
adding extra flags but also extra files to be linked (such as the fileio
open, read, write, ... stubs).

-- 
Julio Guerra