From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andreas Gruenbacher Subject: Re: [RFC] POSIX ACL kernel infrastructure Date: Mon, 5 Aug 2002 14:11:33 +0200 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <200208051411.33286.agruen@suse.de> References: <200208041546.35234.agruen@suse.de> <200208041614.47152.agruen@suse.de> <20020804153349.A28109@infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT Cc: Linux-FSDevel Return-path: To: Christoph Hellwig In-Reply-To: <20020804153349.A28109@infradead.org> List-Id: linux-fsdevel.vger.kernel.org On Sunday 04 August 2002 16:33, Christoph Hellwig wrote: > On Sun, Aug 04, 2002 at 04:14:47PM +0200, Andreas Gruenbacher wrote: > > +MODULE_AUTHOR("Andreas Gruenbacher "); > > +MODULE_DESCRIPTION("Generic Posix Access Control List (ACL) > > Manipulation"); +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0) > > +MODULE_LICENSE("GPL"); > > +#endif > > MODULE_LICENSE was new in 2.4.9 or 2.4.10, but certainly not present in > 2.4.1.. I don't think kernel version checks in core code are a good idea > though. especially if you aim for inclusion. It was 2.4.10; I've canged that. I wanted to use identical code from 2.2 to 2.4; the non-2.5 parts can be stripped off. > > +posix_acl_t * > > +posix_acl_alloc(int count) > > +{ > > + const size_t size = sizeof(posix_acl_t) + > > + count * sizeof(posix_acl_entry_t); > > + posix_acl_t *acl = kmalloc(size, GFP_KERNEL); > > + if (acl) { > > + atomic_set(&acl->a_refcount, 1); > > + acl->a_count = count; > > + } > > + return acl; > > are you sure this is never called from filesystem transactions? > passing a gfp flag down seems like a good idea to me. Probably, yes. > > +/* > > + * Duplicate an ACL handle. > > + */ > > +posix_acl_t * > > +posix_acl_dup(posix_acl_t *acl) > > +{ > > + if (acl) > > + atomic_inc(&acl->a_refcount); > > + return acl; > > +} > > Make this an inline in a header? can acl really be NULL? Yes it can, meaning `there are only the file permission bits'. > > +/* > > + * Get the POSIX ACL of an inode. > > + */ > > +posix_acl_t * > > +get_posix_acl(struct inode *inode, int type) > > +{ > > + posix_acl_t *acl; > > + > > + if (!inode->i_op || !inode->i_op->get_posix_acl) > > + return ERR_PTR(-ENOTSUP); > > inode->i_op is never NULL. Changed. > > + down(&inode->i_sem); > > + lock_kernel(); /* goes away in 2.5.x */ > > this patch _is_ for 2.5, isn't it? Removed. > > linux-2.5.30.patch/include/linux/fs.h > > --- linux-2.5.30/include/linux/fs.h Thu Aug 1 23:16:15 2002 > > +++ linux-2.5.30.patch/include/linux/fs.h Sun Aug 4 13:29:31 2002 > > @@ -23,6 +23,7 @@ > > #include > > #include > > #include > > +#include > > > > #include > > > > @@ -787,6 +788,8 @@ > > ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t); > > ssize_t (*listxattr) (struct dentry *, char *, size_t); > > int (*removexattr) (struct dentry *, const char *); > > + posix_acl_t *(*get_posix_acl) (struct inode *, int); > > + int (*set_posix_acl) (struct inode *, int, posix_acl_t *); > > If you had followed Documentation/CodingSyle and use struct posix_acl > instead of posix_acl_t we wouldn't have to bloat fs.h with yet another > indirect header.. Using `struct posix_acl' everywhere is so much more messy. Is an exception justified here? The core kernel code has typedefs all over the place. > Also what exactly are get_posix_acl/set_posix_acl for? We have wrappers > for them in fs/posix_acl.c, but even in your 2.4 patch only get_posix_acl > is ever used. Shouldn't we always set/get posix ACLs through the xattr > inode operations? >>From user space, yes. The get_posix_acl operation is currently used in nfsd; going via the xattr operations would be too expensive here. The set_posix_acl operation is indeed not used so far; I think it makes sense to add it for completeness' sake. > > > +#ifdef __KERNEL__ > > why the __KERNEL__? Not needed anymore. > > +/* pxacl.c */ > > Shouldn't this be posix_acl.c? Yes. I have put up an updated version at . Regards, Andreas. ------------------------------------------------------------------ Andreas Gruenbacher SuSE Linux AG mailto:agruen@suse.de Deutschherrnstr. 15-19 http://www.suse.de/ D-90429 Nuernberg, Germany