> On Dec 26, 2023, at 11:57 AM, Konstantin Belousov <[email protected]> wrote:
>
> On Tue, Dec 26, 2023 at 11:31:38AM +0800, Zhenlei Huang wrote:
>>
>>
>>> On Dec 26, 2023, at 11:29 AM, Zhenlei Huang <[email protected]> wrote:
>>>
>>>
>>>
>>>> On Dec 26, 2023, at 9:36 AM, Konstantin Belousov <[email protected]
>>>> <mailto:[email protected]>> wrote:
>>>>
>>>> The branch main has been updated by kib:
>>>>
>>>> URL:
>>>> https://cgit.FreeBSD.org/src/commit/?id=2a1d50fc12f6e604da834fbaea961d412aae6e85
>>>>
>>>> <https://cgit.freebsd.org/src/commit/?id=2a1d50fc12f6e604da834fbaea961d412aae6e85>
>>>>
>>>> commit 2a1d50fc12f6e604da834fbaea961d412aae6e85
>>>> Author: Andrew Gierth <[email protected]
>>>> <mailto:[email protected]>>
>>>> AuthorDate: 2023-12-24 12:04:21 +0000
>>>> Commit: Konstantin Belousov <[email protected]
>>>> <mailto:[email protected]>>
>>>> CommitDate: 2023-12-26 01:35:46 +0000
>>>>
>>>> vfs_domount_update(): correct fsidcmp() usage
>>>>
>>>> MFC after: 3 days
>>>> ---
>>>> sys/kern/vfs_mount.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c
>>>> index 8e54c832e9f1..331e4887c200 100644
>>>> --- a/sys/kern/vfs_mount.c
>>>> +++ b/sys/kern/vfs_mount.c
>>>> @@ -1388,7 +1388,7 @@ vfs_domount_update(
>>>> error = EINVAL;
>>>> goto end;
>>>> }
>>>> - if (fsidcmp(&fsid_up, &mp->mnt_stat.f_fsid) != 0) {
>>>> + if (fsidcmp(fsid_up, &mp->mnt_stat.f_fsid) != 0) {
>>>> error = ENOENT;
>>>> goto end;
>>>> }
>>>
>>>
>>
>> The fsidcmp is currently defined as
>> ```
>> #define fsidcmp(a, b) memcmp((a), (b), sizeof(fsid_t))
>> ```
> Yes, and the issue should be fixed by making it (inline) function, which
> automatically would type-check it args.
An inline function sound much better (than macro). It is also simple ;)
>
>>
>>>
>>> I guess we want to ensure ` typeof(a) == typeof(b) == fsid_t `, to prevent
>>> such kind of error in future.
>>>
>>> I see both gcc [1] and clang [2] have __builtin_types_compatible_p , so
>>> probably a good definition of fsidcmp should be
>>>
>>> ```
>>> #define TYPEASSERT(v, t) \
>>> _Static_assert(__builtin_types_compatible_p(typeof(v), t), "Requires
>>> type '" #t "'")
>>>
>>> #define fsidcmp(a, b) ({ \
>>> TYPEASSERT((a), fsid_t *); \
>>> TYPEASSERT((b), fsid_t *); \
>>> memcmp((a), (b), sizeof(fsid_t)); \
>>> })
>>> ```
>>>
>>> 1. https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
>>> <https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html>
>>> 2. https://clang.llvm.org/docs/LanguageExtensions.html
>>> <https://clang.llvm.org/docs/LanguageExtensions.html>
>>> Best regards,
>>> Zhenlei
>>>
>>
>>
