posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Maxim Sobolev]

Hi, while working on some unrelated feature I've noticed that at least
those two system calls are not returning proper value (-1) on error.
Instead actual errno value is returned from the syscall verbatim,
i.e. posix_fadvise() returns 22 on EINVAL. Attached patch fixes that
problem, however I am not sure if I need to assign td->td_retval[0] at all,
those two operations by design never return anything but -1 on error and 0
on success. Can someone comment on this? Thanks!
diff --git a/sys/kern/vfs_syscalls.c b/sys/kern/vfs_syscalls.c
index e675b09..bdb1639 100644
--- a/sys/kern/vfs_syscalls.c
+++ b/sys/kern/vfs_syscalls.c
@@ -4528,7 +4528,7 @@ sys_posix_fallocate(struct thread *td, struct 
posix_fallocate_args *uap)
 
td->td_retval[0] = kern_posix_fallocate(td, uap->fd, uap->offset,
uap->len);
-   return (0);
+   return (td->td_retval[0]);
 }
 
 /*
@@ -4665,5 +4665,5 @@ sys_posix_fadvise(struct thread *td, struct 
posix_fadvise_args *uap)
 
td->td_retval[0] = kern_posix_fadvise(td, uap->fd, uap->offset,
uap->len, uap->advice);
-   return (0);
+   return (td->td_retval[0]);
 }
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Dag-Erling Smørgrav]

Maxim Sobolev  writes:
> Hi, while working on some unrelated feature I've noticed that at least
> those two system calls are not returning proper value (-1) on error.
> Instead actual errno value is returned from the syscall verbatim,
> i.e. posix_fadvise() returns 22 on EINVAL.

That's how syscalls work.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Maxim Sobolev]

Ah, ok, I see now. It's been broken and still broken in 9.x/10.x, already
fixed in trunk and I been just reading wrong manpage. Thanks for the
pointer, on a related note those fixes should probably be MFCed into 10.3
if it has not been already.

On Tue, Dec 8, 2015 at 9:42 AM, Konstantin Belousov 
wrote:

> On Tue, Dec 08, 2015 at 04:52:05PM +0100, Dag-Erling Sm??rgrav wrote:
> > Maxim Sobolev  writes:
> > > Hi, while working on some unrelated feature I've noticed that at least
> > > those two system calls are not returning proper value (-1) on error.
> > > Instead actual errno value is returned from the syscall verbatim,
> > > i.e. posix_fadvise() returns 22 on EINVAL.
> >
> > That's how syscalls work.
>
> No, this is not how typical syscalls work, but is how the posix_fallocate()
> and posix_fadvise() are specified by Posix.  The patch is wrong, see also
> r261080 and r288640.
>
>
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Garrett Wollman]

In article 
sobo...@freebsd.org writes:

>Hi, while working on some unrelated feature I've noticed that at least
>those two system calls are not returning proper value (-1) on error.
>Instead actual errno value is returned from the syscall verbatim,

That is what the specification requires.

RETURN VALUE
Upon successful completion, posix_fadvise( ) shall return
zero; otherwise, an error number shall be returned to
indicate the error.

(Quote from SUSv7 p. 1410, lines 46221-46223.)

-GAWollman

___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Maxim Sobolev]

Then it's documentation bug or maybe some discrepancy between SUS and
POSIX.

$ man posix_fadvise
RETURN VALUES
 The posix_fadvise() function returns the value 0 if successful;
otherwise
 the value -1 is returned and the global variable errno is set to
indicate
 the error.
STANDARDS
 The posix_fadvise() interface conforms to IEEE Std 1003.1-2001
 (``POSIX.1'').

HISTORY
 The posix_fadvise() system call first appeared in FreeBSD 9.1.


On Tue, Dec 8, 2015 at 9:01 AM, Garrett Wollman <
woll...@hergotha.csail.mit.edu> wrote:

> In article  nfgc...@mail.gmail.com>
> sobo...@freebsd.org writes:
>
> >Hi, while working on some unrelated feature I've noticed that at least
> >those two system calls are not returning proper value (-1) on error.
> >Instead actual errno value is returned from the syscall verbatim,
>
> That is what the specification requires.
>
> RETURN VALUE
> Upon successful completion, posix_fadvise( ) shall return
> zero; otherwise, an error number shall be returned to
> indicate the error.
>
> (Quote from SUSv7 p. 1410, lines 46221-46223.)
>
> -GAWollman
>
>
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Mark Johnston]

On Tue, Dec 08, 2015 at 01:35:31AM -0800, Maxim Sobolev wrote:
> Hi, while working on some unrelated feature I've noticed that at least
> those two system calls are not returning proper value (-1) on error.
> Instead actual errno value is returned from the syscall verbatim,
> i.e. posix_fadvise() returns 22 on EINVAL. Attached patch fixes that
> problem, however I am not sure if I need to assign td->td_retval[0] at all,
> those two operations by design never return anything but -1 on error and 0
> on success. Can someone comment on this? Thanks!

This behaviour is documented and specified by POSIX. I'm not sure why
these syscalls are inconsistent with everything else, but the current
implementation is correct.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Konstantin Belousov]

On Tue, Dec 08, 2015 at 04:52:05PM +0100, Dag-Erling Sm??rgrav wrote:
> Maxim Sobolev  writes:
> > Hi, while working on some unrelated feature I've noticed that at least
> > those two system calls are not returning proper value (-1) on error.
> > Instead actual errno value is returned from the syscall verbatim,
> > i.e. posix_fadvise() returns 22 on EINVAL.
> 
> That's how syscalls work.

No, this is not how typical syscalls work, but is how the posix_fallocate()
and posix_fadvise() are specified by Posix.  The patch is wrong, see also
r261080 and r288640.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Dag-Erling Smørgrav]

Konstantin Belousov  writes:
> Dag-Erling Smørgrav  writes:
> > Maxim Sobolev  writes:
> > > Hi, while working on some unrelated feature I've noticed that at least
> > > those two system calls are not returning proper value (-1) on error.
> > > Instead actual errno value is returned from the syscall verbatim,
> > > i.e. posix_fadvise() returns 22 on EINVAL.
> > That's how syscalls work.
> No, this is not how typical syscalls work, but is how the posix_fallocate()
> and posix_fadvise() are specified by Posix.  The patch is wrong, see also
> r261080 and r288640.

Umm, I can't find the code ATM but syscalls store the actual return
value in td_retval and return 0 or EWHATEVER and the syscall wrapper
handles the translation.  If that's not what Maxim was talking about,
then please ignore me.

Anyway, happy to hear that the X/Open group have found a new way to
screw people over.

DES
-- 
Dag-Erling Smørgrav - d...@des.no
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: posix_fallocate(2) && posix_fadvise(2) are somewhat broken

[Konstantin Belousov]

On Tue, Dec 08, 2015 at 07:54:06PM +0100, Dag-Erling Sm??rgrav wrote:
> Konstantin Belousov  writes:
> > Dag-Erling Sm??rgrav  writes:
> > > Maxim Sobolev  writes:
> > > > Hi, while working on some unrelated feature I've noticed that at least
> > > > those two system calls are not returning proper value (-1) on error.
> > > > Instead actual errno value is returned from the syscall verbatim,
> > > > i.e. posix_fadvise() returns 22 on EINVAL.
> > > That's how syscalls work.
> > No, this is not how typical syscalls work, but is how the posix_fallocate()
> > and posix_fadvise() are specified by Posix.  The patch is wrong, see also
> > r261080 and r288640.
> 
> Umm, I can't find the code ATM but syscalls store the actual return
> value in td_retval and return 0 or EWHATEVER and the syscall wrapper
> handles the translation.  If that's not what Maxim was talking about,
> then please ignore me.
I mean that typical syscall does not return error to usermode, it
returns -1 and sets errno. But usermode conventions for the posix_f*e()
are different, and I believe this is what tripped over Maxim and I
reacted upon.

Indeed kernel expects the syscall function from the sysentvec table
to return error or zero. If zero is returned, then td_retval array is
translated into return value for usermode by cpu_set_syscall_retval().
If non-zero is returned, typical kernel/libc interface returns the
syscall function return value to usermode and additionally set flag
(like PSL_C in the processor status word). Of course, there is an
additional translation layer in usermode syscall trampolines.


> 
> Anyway, happy to hear that the X/Open group have found a new way to
> screw people over.
It is the same as the pthread_* conventions.  They are somewhat consistent.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"