This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix misreported errno on preadv2/pwritev2 (BZ#23579)



On 28/09/2018 11:37, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 28/09/2018 11:10, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> 	[BZ #23579]
>>>> 	* misc/tst-preadvwritev2-common.c (do_test_with_invalid_fd): New
>>>> 	test.
>>>> 	* misc/tst-preadvwritev2.c, misc/tst-preadvwritev64v2.c (do_test):
>>>> 	Call do_test_with_invalid_fd.
>>>> 	* sysdeps/unix/sysv/linux/preadv2.c (preadv2): Use fallback code iff
>>>> 	errno is ENOSYS.
>>>> 	* sysdeps/unix/sysv/linux/preadv64v2.c (preadv64v2): Likewise.
>>>> 	* sysdeps/unix/sysv/linux/pwritev2.c (pwritev2): Likewise.
>>>> 	* sysdeps/unix/sysv/linux/pwritev64v2.c (pwritev64v2): Likewise.
>>>
>>> Looks okay to me.
>>>
>>> I see that we return ENOTSUP for invalid flags, but the kernel returns
>>> EINVAL.  But this is a preexisting problem, unrelated to this patch.
>>
>> Yeah, I described this discrepancy in commit message and I can't really see
>> how to easy emulate it.
> 
> I think there are two issues: EBADF vs flag error, and that the flag
> error is ENOTSUP, not EINVAL as the kernel returns (according to the
> manual page at least).  The second part would be fixable, but not the
> first part.

The EINVAL for invalid flags is not what I am seeing with 4.15.0:

preadv2(3, [{iov_base=0x7ffcb69d1620, iov_len=32}], 1, 0, 0x20 /* RWF_??? */) = -1 EOPNOTSUPP (Operation not supported)
pwritev2(3, [{iov_base="\20\0\0\0\0\0\0\0\254\205\256[\0\0\0\0P\255(\16\0\0\0\0\254\205\256[\0\0\0\0", iov_len=32}], 1, 0, 0x20 /* RWF_??? */) = -1 EOPNOTSUPP (Operation not supported)

And this I confirmed that is what kernels is doing:

pwritev2
\_ do_writev | do_pwritev
   \_ vfs_writev
      \_ do_iter_write
         \_ do_iter_readv_writev
            \_ kiocb_set_rw_flags

 665 static ssize_t do_iter_readv_writev(struct file *filp, struct iov_iter *iter,
 666                 loff_t *ppos, int type, rwf_t flags)
 667 {
 668         struct kiocb kiocb;
 669         ssize_t ret;
 670 
 671         init_sync_kiocb(&kiocb, filp);
 672         ret = kiocb_set_rw_flags(&kiocb, flags);
 673         if (ret)
 674                 return ret;

static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags)
{
        if (unlikely(flags & ~RWF_SUPPORTED))
                return -EOPNOTSUPP;


         \_ do_loop_readv_writev

 687 static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,        
 688                 loff_t *ppos, int type, rwf_t flags)                                 
 689 {       
 690         ssize_t ret = 0;                                                             
 691         
 692         if (flags & ~RWF_HIPRI)
 693                 return -EOPNOTSUPP;     

I think we should update man-pages regarding this behaviour.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]