Eygene Ryabinkin <rea-f...@codelabs.ru> writes:
> For the current code state, check "*cp == '\0'" seems to be redundant:
> [...]  By the way, comment before the test "if (rdonly)' seems to be
> slightly misleading [...]

OK, I see that.  I've removed the check and rewritten the comment.

What about this "temporary assert"?

                /*
                 * This is a temporary assert to make sure I know what the
                 * behavior here was.
                 */
                KASSERT((cnp->cn_flags & (WANTPARENT|LOCKPARENT)) != 0,
                   ("lookup: Unhandled case."));

It was added by jeff@ four years ago, with the following log message:

 - Add a few asserts for some unusual conditions that I do not believe can
   happen.  These will later go away and turn into implementations for these
   conditions.

Any reason not to remove it?

> Seems like here we can also check for the trailing slash to be set on
> any previous invocation (or the current one), since we're following
> symlinks in the case of directories, so we should follow them to the
> end.

I'm not sure I understand...

> Perhaps 'XXX for direnter()' should be changed to something like
> 'strip trailing slashes in cnp->cn_nameptr'.

I'll just remove it, since the previous comment clearly explains what is
going on.

> By the way, comment just after 'nextname' label is misleading: we can be
> there with symbolic link, but it won't be followed.  So "Not a symbolic
> link" can be changed to something like "Not a symbolic link that will
> be followed".

        /*
         * Not a symbolic link that we will follow.  Continue with the
         * next component if there is any; otherwise, we're done.
         */

DES
-- 
Dag-Erling Smørgrav - d...@des.no
_______________________________________________
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"

Reply via email to