vfs posix compatibility change

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

vfs posix compatibility change

Moritz Buhl-3
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 -0000 1.80
+++ sys/kern/vfs_lookup.c 22 Jul 2019 16:10:14 -0000
@@ -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 -0000 1.22
+++ lib/libc/sys/rename.2 22 Jul 2019 16:45:16 -0000
@@ -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

Reply | Threaded
Open this post in threaded view
|

Re: vfs posix compatibility change

Moritz Buhl-3
Any comments?

July 22, 2019 8:23 PM, "Moritz Buhl" <[hidden email]> 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 -0000 1.80
> +++ sys/kern/vfs_lookup.c 22 Jul 2019 16:10:14 -0000
> @@ -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 -0000 1.22
> +++ lib/libc/sys/rename.2 22 Jul 2019 16:45:16 -0000
> @@ -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