Re: [RFC] POSIX ACL kernel infrastructure
flat view
From: Christoph Hellwig <hch@infradead.org>
Date: 2002-08-04 14:33:49
On Sun, Aug 04, 2002 at 04:14:47PM +0200, Andreas Gruenbacher wrote:
quoted hunk
+MODULE_AUTHOR("Andreas Gruenbacher <a.gruenbacher@computer.org>"); +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.
quoted hunk
+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 filsystem transactions? passing a gfp flag down seems like a good idea to me.
quoted hunk
+/* + * 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?
quoted hunk
+/* + * 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.
+ down(&inode->i_sem); + lock_kernel(); /* goes away in 2.5.x */
this patch _is_ for 2.5, isn't it?
quoted hunk
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 <linux/string.h> #include <linux/radix-tree.h> #include <linux/bitops.h> +#include <linux/posix_acl.h> #include <asm/atomic.h>@@ -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 osix_acl instead of posix_acl_t we wouldn't have to bloat fs.h with yet another indirect header.. 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?
+#ifdef __KERNEL__
why the __KERNEL__?
+/* pxacl.c */
Shouldn't this be posix_acl.c?