Re: vfs posix compatibility change

2019-09-08 Thread Moritz Buhl
Any comments?

July 22, 2019 8:23 PM, "Moritz Buhl"  wrote:

> Hi,
> 
> here is another POSIX compatibility change I found while running
> NetBSD system call tests:
> 
> The incompatibilities can be reproduced with the followding comands:
> `mv / foo`
> mv: rename / to foo: Is a directory
> `man 2 rename` mentions EISDIR:
> [EISDIR] to is a directory, but from is not a directory.
> Which is mentioned similarly in POSIX. But it doesn't fit the current case.
> 
> `mkdir /`
> mkdir: /: Is a directory
> EISDIR is not mentioned in `man 2 mkdir`. Also not mentioned in POSIX.
> I believe EBUSY would be good here.
> 
> `rmdir /`
> rmdir: /: Is a directory
> EISDIR is not mentioned in `man 2 rmdir`. Also not mentioned in POSIX.
> 
> unlink("/");
> printf("%d\n", errno);
> 21 // EISDIR
> Also neither mentioned in man nor POSIX.
> 
> Here is some POSIX references:
> https://pubs.opengroup.org/onlinepubs/9699919799
> unlink (2):
> [EBUSY]
> The file named by the path argument cannot be unlinked because it is
> being used by the system or another process and the implementation
> considers this an error.
> 
> rename (2):
> [EBUSY]
> The directory named by old or new is currently in use by the system or
> another process, and the implementation considers this an error.
> 
> rmdir (2):
> [EBUSY]
> The directory to be removed is currently in use by the system or
> some process and the implementation considers this to be an error.
> 
> mkdir (2):
> [EEXIST]
> The named file exists.
> 
> Attached is a patch that will make mkdir return EEXIST, EBUSY for others.
> I believe this corer case is quite essential as it is in namei because of
> this (and because NetBSD and FreeBSD differ a lot in the vfs area) I hope
> for some comments on this.
> 
> The patched code hasn't changed a lot since 44BSD. duh POSIX.
> There already were discussions on EISDIR in the past: (from unlink POSIX)
> 
>> The standard developers reviewed TR 24715-2006 and noted that LSB-conforming 
>> implementations may
>> return [EISDIR] instead of [EPERM] when unlinking a directory. A change to 
>> permit this behavior by
>> changing the requirement for [EPERM] to [EPERM] or [EISDIR] was considered, 
>> but decided against
>> since it would break existing strictly conforming and conforming 
>> applications. Applications written
>> for portability to both POSIX.1-2017 and the LSB should be prepared to 
>> handle either error code.
> 
> duh.
> 
> thanks,
> mbuhl
> 
> Index: sys/kern/vfs_lookup.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_lookup.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 vfs_lookup.c
> --- sys/kern/vfs_lookup.c 18 Jul 2019 18:06:17 - 1.80
> +++ sys/kern/vfs_lookup.c 22 Jul 2019 16:10:14 -
> @@ -440,7 +440,10 @@ vfs_lookup(struct nameidata *ndp)
> */
> if (cnp->cn_nameptr[0] == '\0') {
> if (ndp->ni_dvp == NULL && wantparent) {
> - error = EISDIR;
> + if (cnp->cn_nameiop == CREATE)
> + error = EEXIST;
> + else
> + error = EBUSY;
> goto bad;
> }
> ndp->ni_vp = dp;
> Index: lib/libc/sys/rename.2
> ===
> RCS file: /cvs/src/lib/libc/sys/rename.2,v
> retrieving revision 1.22
> diff -u -p -r1.22 rename.2
> --- lib/libc/sys/rename.2 10 Sep 2015 17:55:21 - 1.22
> +++ lib/libc/sys/rename.2 22 Jul 2019 16:45:16 -
> @@ -123,6 +123,12 @@ characters, or an entire pathname (inclu
> exceeded
> .Dv PATH_MAX
> bytes.
> +.It Bq Er EBUSY
> +The directory containing
> +.Fa from
> +or
> +.Fa to
> +is currently in use by the system.
> .It Bq Er ENOENT
> A component of the
> .Fa from



vfs posix compatibility change

2019-07-22 Thread Moritz Buhl
Hi,

here is another POSIX compatibility change I found while running
NetBSD system call tests:

The incompatibilities can be reproduced with the followding comands:
`mv / foo`
mv: rename / to foo: Is a directory
`man 2 rename` mentions EISDIR:
[EISDIR]   to is a directory, but from is not a directory.
Which is mentioned similarly in POSIX. But it doesn't fit the current case.

`mkdir /`
mkdir: /: Is a directory
EISDIR is not mentioned in `man 2 mkdir`. Also not mentioned in POSIX.
I believe EBUSY would be good here.

`rmdir /`
rmdir: /: Is a directory
EISDIR is not mentioned in `man 2 rmdir`. Also not mentioned in POSIX.

unlink("/");
printf("%d\n", errno);
21 // EISDIR
Also neither mentioned in man nor POSIX.

Here is some POSIX references:
https://pubs.opengroup.org/onlinepubs/9699919799/
unlink (2):
[EBUSY]
The file named by the path argument cannot be unlinked because it is
being used by the system or another process and the implementation
considers this an error.

rename (2):
[EBUSY]
The directory named by old or new is currently in use by the system or
another process, and the implementation considers this an error.

rmdir (2):
[EBUSY]
The directory to be removed is currently in use by the system or
some process and the implementation considers this to be an error.

mkdir (2):
[EEXIST]
The named file exists.

Attached is a patch that will make mkdir return EEXIST, EBUSY for others.
I believe this corer case is quite essential as it is in namei because of
this (and because NetBSD and FreeBSD differ a lot in the vfs area) I hope
for some comments on this.

The patched code hasn't changed a lot since 44BSD. duh POSIX.
There already were discussions on EISDIR in the past: (from unlink POSIX)

> The standard developers reviewed TR 24715-2006 and noted that LSB-conforming 
> implementations may return [EISDIR] instead of [EPERM] when unlinking a 
> directory. A change to permit this behavior by changing the requirement for 
> [EPERM] to [EPERM] or [EISDIR] was considered, but decided against since it 
> would break existing strictly conforming and conforming applications. 
> Applications written for portability to both POSIX.1-2017 and the LSB should 
> be prepared to handle either error code.

duh.

thanks,
mbuhl

Index: sys/kern/vfs_lookup.c
===
RCS file: /cvs/src/sys/kern/vfs_lookup.c,v
retrieving revision 1.80
diff -u -p -r1.80 vfs_lookup.c
--- sys/kern/vfs_lookup.c   18 Jul 2019 18:06:17 -  1.80
+++ sys/kern/vfs_lookup.c   22 Jul 2019 16:10:14 -
@@ -440,7 +440,10 @@ vfs_lookup(struct nameidata *ndp)
 */
if (cnp->cn_nameptr[0] == '\0') {
if (ndp->ni_dvp == NULL && wantparent) {
-   error = EISDIR;
+   if (cnp->cn_nameiop == CREATE)
+   error = EEXIST;
+   else
+   error = EBUSY;
goto bad;
}
ndp->ni_vp = dp;
Index: lib/libc/sys/rename.2
===
RCS file: /cvs/src/lib/libc/sys/rename.2,v
retrieving revision 1.22
diff -u -p -r1.22 rename.2
--- lib/libc/sys/rename.2   10 Sep 2015 17:55:21 -  1.22
+++ lib/libc/sys/rename.2   22 Jul 2019 16:45:16 -
@@ -123,6 +123,12 @@ characters, or an entire pathname (inclu
 exceeded
 .Dv PATH_MAX
 bytes.
+.It Bq Er EBUSY
+The directory containing
+.Fa from
+or
+.Fa to
+is currently in use by the system.
 .It Bq Er ENOENT
 A component of the
 .Fa from