Re: [PATCH 2/2 V2] fat: Convert to new mount api

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Eric Sandeen <sandeen@redhat.com> writes:

> On 7/1/24 12:35 PM, Eric Sandeen wrote:
>> On 7/1/24 9:15 AM, OGAWA Hirofumi wrote:
>>> Eric Sandeen <sandeen@redhat.com> writes:
> I don't think that will work.
>
> For example, for the allow_utime adjustment...
>
> Before parsing begins, allow_utime is defaulted to -1 (unset) and
> fs_dmask is defaulted to current_umask()
>
> If we put the 
>
> +	if (opts->allow_utime == (unsigned short)-1)
> +		opts->allow_utime = ~opts->fs_dmask & (S_IWGRP | S_IWOTH);
>
> test at the bottom of parse_param, then this sequence of parsing:
>
> ("mount -o fs_uid=42,fs_dmask=0XYZ")
>
> fs_uid=42
>  --> sets opts->allow_utime to (~opts->fs_dmask & (S_IWGRP | S_IWOTH))
>      where fs_dmask is default / current_umask()
> fs_dmask=0XYZ
>  --> changes fs_dmask from default, but does not update allow_utime which
>      was set based on the old fs_dmask
>
> leads to different results than:
>
> ("mount -o fs_dmask=0XYZ",fs_uid=42)
>
> fs_dmask=0XYZ
>  --> changes fs_dmask from the default
>      updates allow_utime based on this user-specified fs_dmask rather than default
> fs_uid=42
>  --> allow_utime is now set, so no further changes are made
>
> IOWS, the final allow_utime value may differ depending on the order of option
> parsing, unless we wait until parsing is complete before we inspect and adjust it.
>
> dhowells did, however, suggest that perhaps these adjustments should generally
> be done in get_tree rather than fill_super, so I'll give that a shot.
>
> Sound ok?

Ah, you are right. I was misread how parser works. And then your
original may be readable than get_tree.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>