Re: [issue1701] mkdir / returns EPERM instead of EEXIST (related to pkg_add)

[Chris Turner <c.turner@xxxxxxxxxxxxxxxxxxx>]

Stathis Kamperis (via DragonFly issue tracker) wrote:
> Stathis Kamperis <ekamperi@gmail.com> added the comment:
>
> Hey all.
>
> First, EPERM shouldn't be returned at all; instead EACCES should.
>

This indeed looks to be related to the exact section of code Alex 
mentioned - see attached for (trivial) 'mkdir' related fix. (1LOC + 
comment update)

In the current code comments (e.g. pre-this-patch), 'rmdir /' is 
explicitly mentioned, so assuming this is the related reference,
POSIX specifies that EPERM or EACCESS is OK.

http://www.opengroup.org/onlinepubs/000095399/functions/rmdir.html

There is another section of code further down, related to non-edge 
cases? labeled 'POSIX junk', of note.

So - anyhow - assuming that this terribly exiting and non-pedantic 
conversation about standards conformance is resolved in short order - 
should I commit this to the just-cut release branch or no?

(mea culpa on always having the 5min late patches. story-of-life. sigh)

Did 5-6 rudimentary tests in a vkernel, along the lines of:

mkdir / -> eexist
rmdir / -> eperm
mkdir /tmp/foo -> works!
mkdir /tmp/foo -> eexist
rmdir /tmp/foo -> works!

I'm not familiar enough with namecache users to suggest a call here ..

Cheers

- Chris

> Second, POSIX says that the order of error detection is undefined. 
So, we may
> choose to check first the permissions and then if directory exists, 
or the other
> way around. Both are valid.

nothing to say about this meself..

diff --git a/sys/kern/vfs_nlookup.c b/sys/kern/vfs_nlookup.c
index ab7df39..4d7141b 100644
--- a/sys/kern/vfs_nlookup.c
+++ b/sys/kern/vfs_nlookup.c
@@ -469,14 +469,15 @@ nlookup(struct nlookupdata *nd)
 	    nd->nl_nch = nch;		/* remains locked */
 
 	    /*
-	     * Fast-track termination.  There is no parent directory of
-	     * the root in the same mount from the point of view of
-	     * the caller so return EPERM if NLC_REFDVP is specified.
-	     * e.g. 'rmdir /' is not allowed.
+	     * Fast-track termination. In the case of '/', there is no 
+	     * parent directory from the point of view of the caller 
+	     * so return either EPERM if NLC_REFDVP is specified.
+	     * e.g. 'rmdir /' is not allowed, or return EEXIST
+	     * for the case of 'mkdir /'.
 	     */
 	    if (*ptr == 0) {
 		if (nd->nl_flags & NLC_REFDVP)
-			error = EPERM;
+			error = (nd->nl_flags & NLC_CREATE) ? EEXIST : EPERM;
 		else
 			error = 0;
 		break;