Messages in this thread | | | From | Andreas Gruenbacher <> | Date | Mon, 30 Nov 2020 14:09:23 +0100 | Subject | Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build |
| |
On Mon, Nov 30, 2020 at 5:29 AM Randy Dunlap <rdunlap@infradead.org> wrote: > On 11/29/20 7:37 PM, Sergey Senozhatsky wrote: > > A quick question, shouldn't there be dummy definitions for > > the EXPORT_SYMBOL-s? So that external modules can be modprobed > > and used. > > > > Some of posix_acl exported symbols have dummy definitions, > > others don't. > > > > E.g. posix_acl_create() is exported symbol and it's defined for > > both FS_POSIX_ACL and !FS_POSIX_ACL. While exported set_posix_acl() > > is defined only for FS_POSIX_ACL config.
This is to keep the amount of ifdefs in the code reasonably low: by defining posix_acl_create as a dummy inline function like that, inode creation in filesystems can be implemented without any ifdefs as in jffs2_init_acl_pre whether or not CONFIG_FS_POSIX_ACL is enabled, for example. Have a look at different filesystems to see how they avoid using POSIX ACL code when that feature is disabled.
Note that ext2 / ext4 could be built without POSIX ACL support in the past. That's at least broken since the following two commits though:
commit 59fed3bf8a461 ("ext2: cache NULL when both default_acl and acl are NULL") commit 6fd941784b8ac ("ext4: cache NULL when both default_acl and acl are NULL")
> Hi, > > Currently CONFIG_FS_POSIX_ACL differences seem to be handled in > each source file as needed: > > fs/inode.c:#ifdef CONFIG_FS_POSIX_ACL > fs/inode.c:#ifdef CONFIG_FS_POSIX_ACL > fs/namei.c:#ifdef CONFIG_FS_POSIX_ACL > fs/overlayfs/dir.c: if (!IS_ENABLED(CONFIG_FS_POSIX_ACL) || !acl) > fs/overlayfs/inode.c: if (!IS_ENABLED(CONFIG_FS_POSIX_ACL) || !IS_POSIXACL(realinode)) > fs/overlayfs/inode.c:#ifdef CONFIG_FS_POSIX_ACL > fs/overlayfs/super.c:#ifdef CONFIG_FS_POSIX_ACL > fs/xattr.c:#ifdef CONFIG_FS_POSIX_ACL > include/linux/evm.h:#ifdef CONFIG_FS_POSIX_ACL > include/linux/fs.h:#ifdef CONFIG_FS_POSIX_ACL > include/linux/posix_acl.h:#ifdef CONFIG_FS_POSIX_ACL > include/linux/posix_acl.h:#endif /* CONFIG_FS_POSIX_ACL */ > include/linux/posix_acl_xattr.h:#ifdef CONFIG_FS_POSIX_ACL > > However, I have no objection to your patch. > > I am adding Andreas & Al for their viewpoints.
Sergey, what actual problem is your patch trying to solve? It sounds like this is either theoretical and pointless, or you're trying to build an external module that uses POSIX ACL functions that shouldn't be needed when CONFIG_FS_POSIX_ACL is disabled. In the latter case, the external module will just end up including dead code, so the module should be fixed instead.
Thanks, Andreas
| |