Hi,
This patch does not allow open(name, O_DIRECTORY) to be successful for
non-directories in reiser4. It replaces ->i_op->lookup != NULL "is dir" check
for the last path component by explicit S_ISDIR(->i_mode) check.
Regardless to reiser4, S_ISDIR() looks more clear there.
Thanks in advance.
===== fs/namei.c 1.104 vs edited =====
--- 1.104/fs/namei.c Tue Aug 10 16:59:38 2004
+++ edited/fs/namei.c Sun Sep 12 11:00:18 2004
@@ -816,7 +816,7 @@
break;
if (lookup_flags & LOOKUP_DIRECTORY) {
err = -ENOTDIR;
- if (!inode->i_op || !inode->i_op->lookup)
+ if (!S_ISDIR(inode->i_mode))
break;
}
goto return_base;
On Mon, Sep 13, 2004 at 11:49:22AM +0400, Alex Zarochentsev wrote:
> Hi,
>
> This patch does not allow open(name, O_DIRECTORY) to be successful for
> non-directories in reiser4. It replaces ->i_op->lookup != NULL "is dir" check
> for the last path component by explicit S_ISDIR(->i_mode) check.
>
> Regardless to reiser4, S_ISDIR() looks more clear there.
The only objection here is that right now we are guaranteed that cwd and
root of every task have non-NULL ->lookup(). With your patch all we have
is S_ISDIR().
So we either need to check for non-NULL ->lookup() before the beginning of
loop in link_path_walk() or split the flag in two. I would rather do the
former...
>viro wrote
>On Mon, Sep 13, 2004 at 11:49:22AM +0400, Alex Zarochentsev wrote:
> Hi,
>
> This patch does not allow open(name, O_DIRECTORY) to be successful for
> non-directories in reiser4. It replaces ->i_op->lookup != NULL "is dir"
> check for the last path component by explicit S_ISDIR(->i_mode) check.
>
> Regardless to reiser4, S_ISDIR() looks more clear there.
>
>The only objection here is that right now we are guaranteed that cwd and
>root of every task have non-NULL ->lookup(). With your patch all we have
>is S_ISDIR().
>
>So we either need to check for non-NULL ->lookup() before the beginning of
>loop in link_path_walk() or split the flag in two. I would rather do the
>former...
>?
I'm working on something similar, but with alternate pathname resolution
when the path begins with exactly two slashes. Only pseudocode here because
I do not have access to my box:
In path_lookup()
if (*name == '/') {
if (*(name+1)=='/' && *(name+2)==':') {
name+=3;
nd->flags &= LOOKUP_SLASHSLASH
if (*name!='/')
goto relative;
}
In link_path_walk()
When LOOKUP_SLASHSLASH, handle names as follows:
If this !S_ISDIR()
next.name Behavior
------------------------------------------------------
. Same if i_op && i_op->lookup, else -ENOTDIR.
.. Same.
... Okay if i_op && i_op->lookup, else -ENOTDIR.
otherwise Fails with -ENOENT.
If S_ISDIR()
any Same behavior
I tested the //: & flag code and this works (in my limited testing yesterday
with bash and some test programs).
This should limit "hybrid" file-directories to only one valid subdirectory,
the "special metadata" directory. Thus, if a new/modified app wants to
create/access metadata, it would do something like the following:
# relative path
cd /test
touch foo.txt
mkdir foo.txt/... # FAILS
mkdir //:foo.txt/...
echo JayRandom > //:foo.txt/.../Author
# absolute path
echo blahblah > //:/test/foo.txt/.../Title
mkdir testdir
mkdir //:testdir/...
echo no > //:testdir/.../VirusScan
Yes, this means that a) ... is "removed" from the namespace and b) directory
metadata directories are visible to na?ve applications/users while those for
files are not. But it does provide "metadata-aware" apps/users a consistent
way to access this info for both files and directories without resorting to
openat(). Since SuS provides for "implementation-specific" pathname
resolution when pathnames begin with exactly two slashes this should be
legal(?), if desired.
This doesn't address the issue of "active" metadata objects such as reiser4
provides e.g. foo.txt/metas/perms. That's a different discussion.
David
On Mon, Sep 13, 2004 at 04:34:15PM -0500, David Dabbs wrote:
> I'm working on something similar, but with alternate pathname resolution
> when the path begins with exactly two slashes. Only pseudocode here because
> I do not have access to my box:
>
> if (*name == '/') {
> if (*(name+1)=='/' && *(name+2)==':') {
> name+=3;
Pathname resolution is a hell of a fundamental thing and kludges
like that are too ugly to be acceptable. If you can't make that clean
and have to resort to stuffing "special cases" (read: barfbag of ioctl
magnitude) into the areas that might be unspecified by POSIX, don't do it
at all.
I don't like the amount of handwaving from Hans, but *that* is far
worse. Vetoed.
> viro wrote:
>> if (*name == '/') {
>> if (*(name+1)=='/' && *(name+2)==':') {
>> name+=3;
>
> Pathname resolution is a hell of a fundamental thing and kludges
>like that are too ugly to be acceptable. If you can't make that clean
>and have to resort to stuffing "special cases" (read: barfbag of ioctl
>magnitude) into the areas that might be unspecified by POSIX, don't do it
>at all.
>
Even though SuS allows for implementation-specific resolution for pathnames
starting with "//"? It's kludgy, and I suspected that might be the response,
but I thought I'd float it nonetheless.
>I don't like the amount of handwaving from Hans, but *that* is far
>worse. Vetoed.
Kludgy, yes, but far worse? At least I bothered to take the SuS into
consideration and took the time to try an approach, however kludgy, that
might work within them.
Bilious or not, thanks for the feedback.
david