On Wed, 17 Feb 2010, Poul-Henning Kamp wrote:
In message <20100218044931.s95...@delplex.bde.org>, Bruce Evans writes:
On Wed, 17 Feb 2010, Poul-Henning Kamp wrote:
Log:
Mention EISDIR as a possible errno.
It's not a possible error.
critter phk> cat > a.c
#include <stdio.h>
#include <err.h>
int
main(int argc, char **argv)
{
if (unlink("/"))
err(1, "Told you so");
return (0);
}
critter phk> cc a.c
critter phk> ./a.out
a.out: Told you so: Is a directory
critter phk>
Better fix the kernel bug than break the documentation then.
The bug is very old -- it happens in ~5.2, where the code is a bit easier
to understand:
% int
% kern_unlink(struct thread *td, char *path, enum uio_seg pathseg)
% {
% struct mount *mp;
% struct vnode *vp;
% int error;
% struct nameidata nd;
%
% restart:
% bwillwrite();
% NDINIT(&nd, DELETE, NOFOLLOW | LOCKPARENT | LOCKLEAF, pathseg, path,
% td);
% if ((error = namei(&nd)) != 0)
% return (error);
namei() returns EISDIR for "/" (due to DELETE and and the special handling
of the degenerate case which includes "/" and not much else, else the bug
would affect more cases). Then we return the wrong errno before we test
VDIR.
% vp = nd.ni_vp;
% if (vp->v_type == VDIR)
% error = EPERM;
% else {
Untested fix for ~5.2. Also fixes some style bugs.
% Index: vfs_syscalls.c
% ===================================================================
% RCS file: /home/ncvs/src/sys/kern/vfs_syscalls.c,v
% retrieving revision 1.354
% diff -u -2 -r1.354 vfs_syscalls.c
% --- vfs_syscalls.c 24 Jun 2004 17:22:29 -0000 1.354
% +++ vfs_syscalls.c 17 Feb 2010 20:01:09 -0000
% @@ -1614,10 +1605,11 @@
% restart:
% bwillwrite();
% - NDINIT(&nd, DELETE, LOCKPARENT|LOCKLEAF, pathseg, path, td);
% + NDINIT(&nd, DELETE, NOFOLLOW | LOCKPARENT | LOCKLEAF, pathseg, path,
% + td);
Style fixes:
- spell out NOFOLLOW (NOFOLLOW is 0, so omitting it is just confusing).
This bug is in about 3 other vfs syscalls.
- there are spaces around binary operators in KNF. These spaces are
especially important for the "|" operator since this operator looks
more like an alphanumeric character than most operator symbols, but
they are most often omitted for this operator :-(.
% if ((error = namei(&nd)) != 0)
% - return (error);
% + return (error == EISDIR ? EPERM : error);
Fix the EISDIR bug.
% vp = nd.ni_vp;
% if (vp->v_type == VDIR)
% - error = EPERM; /* POSIX */
% + error = EPERM;
Remove banal/misleading comment. The new fixup needs a comment more than
this, but I don't want to add one.
% else {
% /*
ISTR trying to avoid the special handling for the degenerate case in
namei() and lookup() (2 almost identical copies of it). The correct
fix may be there. From lookup() in ~5.2:
% /*
% * Check for degenerate name (e.g. / or "")
% * which is a way of talking about a directory,
% * e.g. like "/." or ".".
% */
% if (cnp->cn_nameptr[0] == '\0') {
% if (dp->v_type != VDIR) {
% error = ENOTDIR;
% goto bad;
% }
% if (cnp->cn_nameiop != LOOKUP) {
% error = EISDIR;
% goto bad;
% }
Cases with trailing slashes are handled by removing the slash, but
this doesn't work for "/" since we don't want to end up with the fully
degenerate name of "". Cases starting with this fully degnerate name
haven't been permitted since ~1988 when POSIX disallowed it, but the
comment in the code hasn't caught up with this. The comment gives
these cases as examples only, but I think that is another bug and that
this is a complete list of degenerate names so the comment should say
"i.e.,". After catching up with 1988 and removing "" from the list,
only "/" remains. This is not really degenerate and can hopefully be
handled more directly and simply. The != VDIR case in the above might
already be unreachable.
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
