vfs: fix description comment of vfs_lookup() #737
Conversation
Signed-off-by: Igor Ostapenko <pm@igoro.pro>
ihoro
force-pushed
the
fix-vfs-lookup-description-comment
branch
from
d7c1145
to
d541fce
Compare
There was a problem hiding this comment.
Hi, thanks for the submission.
This change looks good, and I am happy to merge it, but one question: did you audit any other details of the comment? The entire comment is quite old (1994/1995), and I believe the high-level description still applies but maybe some other details are stale.
If not it is okay, I will just make a note of it in the commit message before pushing.
|
Actually, this is a good question. I simply corrected obvious thing which caught my eyes. From a bit deeper overview I could propose the subsequent commit of changes. |
| * descended until done, or a symbolic link is encountered. The variable | ||
| * ni_more is clear if the path is completed; it is set to one if a | ||
| * descended until done, or a symbolic link is encountered. The cn_flags | ||
| * has ISLASTCN or'ed if the path is completed or ISSYMLINK or'ed if a | ||
| * symbolic link needing interpretation is encountered. |
There was a problem hiding this comment.
It looks that some time ago it was passed as ni_more variable as a flag, today it works with cn_flags bits instead.
| * the target is returned locked, otherwise it is returned unlocked. | ||
| * When creating or renaming and LOCKPARENT is specified, the target may not | ||
| * be ".". When deleting and LOCKPARENT is specified, the target may be ".". |
There was a problem hiding this comment.
Both statements seem to be outdated, probably it makes sense to simply remove them. As I see, at least two cases (RENAME+dot and DELETE+dot) work another way today - it simply returns with EINVAL regardless locking option.
There was a problem hiding this comment.
let's keep this as a followup... I'm going to merge this, but feel free to followup on this detail
| * | ||
| * Overall outline of lookup: | ||
| * | ||
| * handle degenerate case where name is null string |
There was a problem hiding this comment.
Previously it was mentioned to be done upon each dirloop iteration, but today it's done once before the loop.
| * dirloop: | ||
| * identify next component of name at ndp->ni_cnd.cn_nameptr | ||
| * handle degenerate case where name is null string | ||
| * handle .. special cases related to capabilities, chroot, jail |
There was a problem hiding this comment.
Just to mention a bit more cases related to the well-known subsystems.
sys/kern/vfs_lookup.c
Outdated
| * if .. and crossing mount points and on mounted filesys, find parent | ||
| * call VOP_LOOKUP routine for next component name | ||
| * directory vnode returned in ni_dvp, unlocked unless LOCKPARENT set | ||
| * component vnode returned in ni_vp (if it exists), locked. | ||
| * if result vnode is mounted on and crossing mount points, | ||
| * find mounted on vnode | ||
| * if more components of name, do next level at dirloop | ||
| * if VOP_LOOKUP returned ERELOOKUP, repeat the same level at dirloop |
There was a problem hiding this comment.
Probably, it's useful to mention a case when dirloop iteration can be repeated for the same component name again. This mechanism was added for specific autofs case: 213ed83
Signed-off-by: Igor Ostapenko <pm@igoro.pro>
ihoro
force-pushed
the
fix-vfs-lookup-description-comment
branch
from
6143101
to
f10cecf
Compare
|
7b5a1c3 landed. |
Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: imp, mhorne Pull Request: #737
Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: imp, mhorne Pull Request: #737
Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: imp, mhorne Pull Request: freebsd/freebsd-src#737
Signed-off-by: Igor Ostapenko <pm@igoro.pro> Reviewed by: imp, mhorne Pull Request: freebsd/freebsd-src#737
No description provided.