All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruen@suse.de>
To: Christoph Hellwig <hch@infradead.org>
Cc: jjohansen@suse.de, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [RFD 1/4] Pass no useless nameidata to the create, lookup, and permission IOPs
Date: Sat, 30 Jun 2007 18:13:57 +0200	[thread overview]
Message-ID: <200706301813.58435.agruen@suse.de> (raw)
In-Reply-To: <20070630091302.GA21784@infradead.org>

On Saturday 30 June 2007 11:13, Christoph Hellwig wrote:
> We need something like this, but I don't quite like the way you've done
> it.  First the name is wrong, it's not a nameidata anymore but a lookup
> intent, so it should be named that way, struct lookup_intent.

Sure, that name was pretty random ... lookup_intent has gotten the majority of 
votes so far, and I'm perfectly fine with that.

> Second the macro hackery is more than ugly,  please keep the structures
> separate. With modern gcc it might be possible to embed the lookup_intent
> into the nameidata anonymously.

http://gcc.gnu.org/onlinedocs/gcc-4.2.0/gcc/Unnamed-Fields.html

If we can add the -fms-extensions gcc option we can get rid of the macro, and 
the code becomes pretty clean (as shown below). If we cannot add this option, 
then gcc would puke on ``struct lookup_intent;'' in the definition of struct 
nameidata. The macro is the cleanest way to work around this I could come up 
with, but maybe somebody knows another trick.

--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -14,14 +14,10 @@ struct open_intent {
 
 enum { MAX_NESTED_LINKS = 8 };
 
-struct nameidata {
+struct lookup_intent {
 	struct dentry	*dentry;
 	struct vfsmount *mnt;
-	struct qstr	last;
 	unsigned int	flags;
-	int		last_type;
-	unsigned	depth;
-	char *saved_names[MAX_NESTED_LINKS + 1];
 
 	/* Intent data */
 	union {
@@ -29,6 +25,19 @@ struct nameidata {
 	} intent;
 };
 
+struct nameidata {
+	struct lookup_intent;
+	struct qstr	last;
+	int		last_type;
+	unsigned	depth;
+	char *saved_names[MAX_NESTED_LINKS + 1];
+};

> Also please either remove the dentry from struct lookup_entry or from the
> direct argument list of the functions and methods - there is no need to pass
> this one twice.

The dentry in the lookup_intent of the create inode operation is the parent 
dentry right now, and the child dentry is passed as the separate parameter. I 
would prefer the cleaner interface in which the lookup_intent refers to the 
child dentry as well. (Getting from the child to the parent is trivial.) I 
guess this can go in an incremental patch with the next version of these 
patches.

Thanks,
Andreas

  reply	other threads:[~2007-06-30 16:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-06-26 23:15 [RFD 0/4] AppArmor - Don't pass NULL nameidata to vfs_create/lookup/permission IOPs jjohansen
2007-06-26 23:15 ` [RFD 1/4] Pass no useless nameidata to the create, lookup, and permission IOPs jjohansen
2007-06-27  0:11   ` Erez Zadok
2007-06-30  9:14     ` Christoph Hellwig
2007-06-30  9:13   ` Christoph Hellwig
2007-06-30 16:13     ` Andreas Gruenbacher [this message]
2007-06-26 23:15 ` [RFD 2/4] Never pass a NULL nameidata to vfs_create() jjohansen
2007-06-26 23:15 ` [RFD 3/4] Dont use a NULL nameidata in xattr_permission() jjohansen
2007-06-26 23:15 ` [RFD 4/4] Pass nameidata2 to permission() from nfsd_permission() jjohansen
2007-06-26 23:46 ` [RFD 0/4] AppArmor - Don't pass NULL nameidata to vfs_create/lookup/permission IOPs Trond Myklebust
2007-06-27 20:42   ` Andreas Gruenbacher
2007-06-30  9:15   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200706301813.58435.agruen@suse.de \
    --to=agruen@suse.de \
    --cc=hch@infradead.org \
    --cc=jjohansen@suse.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.