Re: [PATCH 1/1] mount_service: use the mount_flags table instead of declaring our own

[Bernd Schubert <bernd@bsbernd.com>]

On 5/9/26 02:48, Darrick J. Wong wrote:
> On Fri, May 08, 2026 at 10:59:45AM -0700, Darrick J. Wong wrote:
>> On Fri, May 08, 2026 at 06:51:45PM +0200, Bernd Schubert wrote:
>>>
>>>
>>> On 5/8/26 00:14, Darrick J. Wong wrote:
>>>> From: Darrick J. Wong <djwong@kernel.org>
>>>>
>>>> Use the canonical mount flags table in mount_util.c instead of
>>>> opencoding our own version in mount_service.c.
>>>>
>>>> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
>>>> ---
>>>>  util/mount_service.c |   51 +++++++++++++-------------------------------------
>>>>  1 file changed, 13 insertions(+), 38 deletions(-)
>>>>
>>>>
>>>> diff --git a/util/mount_service.c b/util/mount_service.c
>>>> index 8c9747b3928a51..79e5b060ba3356 100644
>>>> --- a/util/mount_service.c
>>>> +++ b/util/mount_service.c
>>>> @@ -1457,73 +1457,48 @@ struct ms_to_mount_map {
>>>>  	unsigned int mount_attr_flag;
>>>>  };
>>>>  
>>>> -static const struct ms_to_mount_map attrs[] = {
>>>> -	{ MS_RDONLY,		MOUNT_ATTR_RDONLY },
>>>> -	{ MS_NOSUID,		MOUNT_ATTR_NOSUID },
>>>> -	{ MS_NODEV,		MOUNT_ATTR_NODEV },
>>>> -	{ MS_NOEXEC,		MOUNT_ATTR_NOEXEC },
>>>> -	{ MS_RELATIME,		MOUNT_ATTR_RELATIME },
>>>> -	{ MS_NOATIME,		MOUNT_ATTR_NOATIME },
>>>> -	{ MS_STRICTATIME,	MOUNT_ATTR_STRICTATIME },
>>>> -	{ MS_NODIRATIME,	MOUNT_ATTR_NODIRATIME },
>>>> -#ifdef MOUNT_ATTR_NOSYMFOLLOW
>>>> -	{ MS_NOSYMFOLLOW,	MOUNT_ATTR_NOSYMFOLLOW },
>>>> -#endif
>>>> -	{ 0, 0 },
>>>> -};
>>>> -
>>>>  static void get_mount_attr_flags(const struct fuse_service_mount_command *oc,
>>>>  				 unsigned int *attr_flags,
>>>>  				 unsigned long *leftover_ms_flags)
>>>>  {
>>>> -	const struct ms_to_mount_map *i;
>>>> +	const struct mount_flags *i;
>>>>  	unsigned int ms_flags = ntohl(oc->ms_flags);
>>>>  	unsigned int mount_attr_flags = 0;
>>>>  
>>>> -	for (i = attrs; i->ms_flag != 0; i++) {
>>>> -		if (ms_flags & i->ms_flag)
>>>> -			mount_attr_flags |= i->mount_attr_flag;
>>>> -		ms_flags &= ~i->ms_flag;
>>>> +	for (i = mount_flags; i->opt != NULL; i++) {
>>>> +		if (!i->on || !(ms_flags & i->flag))
>>>> +			continue;
>>>> +
>>>> +		mount_attr_flags |= i->mount_attr;
>>>> +		ms_flags &= ~i->flag;
>>>>  	}
>>>>  
>>>>  	*leftover_ms_flags = ms_flags;
>>>>  	*attr_flags = mount_attr_flags;
>>>>  }
>>>>  
>>>> -struct ms_to_str_map {
>>>> -	unsigned long ms_flag;
>>>> -	const char *string;
>>>> -};
>>>> -
>>>> -static const struct ms_to_str_map strflags[] = {
>>>> -	{ MS_SYNCHRONOUS,	"sync" },
>>>> -	{ MS_DIRSYNC,		"dirsync" },
>>>> -	{ MS_LAZYTIME,		"lazytime" },
>>>> -	{ 0, 0 },
>>>> -};
>>>> -
>>>>  static int set_ms_flags(struct mount_service *mo, unsigned long ms_flags)
>>>>  {
>>>> -	const struct ms_to_str_map *i;
>>>> +	const struct mount_flags *i;
>>>>  	int ret;
>>>>  
>>>> -	for (i = strflags; i->ms_flag != 0; i++) {
>>>> -		if (!(ms_flags & i->ms_flag))
>>>> +	for (i = mount_flags; i->opt != NULL; i++) {
>>>> +		if (!i->on || !(ms_flags & i->flag))
>>>>  			continue;
>>>>  
>>>> -		ret = fsconfig(mo->fsopenfd, FSCONFIG_SET_FLAG, i->string,
>>>> +		ret = fsconfig(mo->fsopenfd, FSCONFIG_SET_FLAG, i->opt,
>>>>  			       NULL, 0);
>>>>  		if (ret) {
>>>>  			int error = errno;
>>>>  
>>>>  			fprintf(stderr, "%s: set %s option: %s\n",
>>>> -				mo->msgtag, i->string, strerror(error));
>>>> +				mo->msgtag, i->opt, strerror(error));
>>>>  			emit_fsconfig_messages(mo);
>>>>  
>>>>  			errno = error;
>>>>  			return -1;
>>>>  		}
>>>> -		ms_flags &= ~i->ms_flag;
>>>> +		ms_flags &= ~i->flag;
>>>>  	}
>>>>  
>>>>  	/*
>>>>
>>>>
>>>
>>>
>>> Hi Darrick,
>>>
>>> wouldn't it make sense to use the functions from mount_fsmount.c? I can
>>> also clean this up later if you want.
>>
>> Oh, hah!  Yes, I'll revise this patch to use them.
> 
> Update: I tried this, but ran into a problem: the two helpers are in
> lib/mount_fsmount.c, which means the object code ends up in libfuse.so.
> Unfortunately, that means that util/mount_service.c can't use them
> unless they're exported from the .so.
> 
> I don't mind doing that, but I feel that I should check with you if it's
> ok to add that to the public libfuse API before doing that?
> 
> Though I think a better solution is to move ms_flags_to_mount_attrs and
> set_ms_flags to mount_util.c because that's compiled into the util/
> programs.
> 
> How about I do that second thing on Monday?


I would prefer the 3rd option, like for fusermount3

executable('fusermount3', ['fusermount.c', '../lib/mount_util.c',
'../lib/mount_fsmount.c', '../lib/util.c'],


I.e. add lib/mount_fsmount.c to mount_service_sources.


Thanks,
Bernd