Hi, Janne Johansson wrote on Sat, Sep 16, 2023 at 11:49:10AM +0200:
> In case someone wants the change in a diff format, those 5 seconds > of work are available here: > http://c66.it.su.se:8080/obsd/unveil.2.diff How come jj@ forgot we want diffs inline? <scratching my head> > +.It Bq Er ENOTDIR > +.Fa path > +points to a file that is not a directory. That diff is certainly not OK. It is misleading because a "path" argument pointing to a file that is not a directory is actually valid. The problem only occurs when you add a trailing slash to the name of a file that is not a directory, like this: $ touch tmp.txt $ ls -l tmp.txt -rw-r--r-- 1 schwarze schwarze 0 Sep 16 18:00 tmp.txt $ ls -l tmp.txt/ ls: tmp.txt/: Not a directory $ ls -l tmp.txt/foobar ls: tmp.txt/foobar: Not a directory Looking at sys_unveil in /sys/kern/vfs_syscalls.c, i suspect that errno actually comes from a lower layer, likely namei(9), rather than from unveil(2) itself. In library manual pages, we occasionally have sentences like this: execv() may fail and set errno for any of the errors specified for the library function execve(2). The fgetln() function may also fail and set errno for any of the errors specified for the routines fflush(3), malloc(3), read(2), stat(2), or realloc(3). But it doesn't look like we do that in system call manuals, and besides, namei(9) appears to lack the RETURN VALUES section. So, what is our policy with syscall ERRORS sections? For standardized syscalls, POSIX probably needs to be taken into account in addition to our implementation. But for our very own syscalls? Hm... access(2), acct(2), chdir(2), chflags(2), chmod(2), chown(2), chroot(2), execve(2), getfh(2), ktrace(2), link(2), mkdir(2), mkfifo(2), mknod(2), mount(2), open(2), pathconf(2), quotactl(2), readlink(2), rename(2), revoke(2), rmdir(2), stat(2), statfs(2), swapctl(2), symlink(2), truncate(2), unlink(2), utimes(2) all list ENOTDIR, and i don't see any right now taking a path argument that don't. Do we want the following diff? > I did not sort the existing return values in alphabetical order, > there were two out of order in there, but that is a separate > commit I guess. NetBSD has a policy of sorting ERRORS entries alphabetically, we don't. Yours, Ingo Index: unveil.2 =================================================================== RCS file: /cvs/src/lib/libc/sys/unveil.2,v retrieving revision 1.22 diff -u -r1.22 unveil.2 --- unveil.2 6 Sep 2021 08:03:08 -0000 1.22 +++ unveil.2 16 Sep 2023 16:38:24 -0000 @@ -158,6 +158,10 @@ A directory in .Fa path did not exist. +.It Bq Er ENOTDIR +A component of the +.Fa path +prefix is not a directory. .It Bq Er EINVAL An invalid value of .Fa permissions
