Re: [PATCH] posix_acl.h: define missing ACL functions on non-posix-acl build

[Andreas Gruenbacher]

On Mon, Nov 30, 2020 at 5:29 AM Randy Dunlap <rdunlap@xxxxxxxxxxxxx> 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