Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Dec 2023 12:09:47 +0800
From:      Zhenlei Huang <zlei@FreeBSD.org>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org>
Subject:   Re: git: 2a1d50fc12f6 - main - vfs_domount_update(): correct fsidcmp() usage
Message-ID:  <640284C5-ACE1-4D31-9BBB-AF181C0C7632@FreeBSD.org>
In-Reply-To: <ZYpPQoNvAIc81wOu@kib.kiev.ua>
References:  <202312260136.3BQ1aOxq051977@gitrepo.freebsd.org> <72190C27-0D45-4300-BB78-719F5EB510E9@FreeBSD.org> <78061056-76D8-457C-A522-8CE62B481317@FreeBSD.org> <ZYpPQoNvAIc81wOu@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help


> On Dec 26, 2023, at 11:57 AM, Konstantin Belousov =
<kostikbel@gmail.com> wrote:
>=20
> On Tue, Dec 26, 2023 at 11:31:38AM +0800, Zhenlei Huang wrote:
>>=20
>>=20
>>> On Dec 26, 2023, at 11:29 AM, Zhenlei Huang <zlei@FreeBSD.org> =
wrote:
>>>=20
>>>=20
>>>=20
>>>> On Dec 26, 2023, at 9:36 AM, Konstantin Belousov <kib@FreeBSD.org =
<mailto:kib@FreeBSD.org>> wrote:
>>>>=20
>>>> The branch main has been updated by kib:
>>>>=20
>>>> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D2a1d50fc12f6e604da834fbaea961d41=
2aae6e85 =
<https://cgit.freebsd.org/src/commit/?id=3D2a1d50fc12f6e604da834fbaea961d4=
12aae6e85>
>>>>=20
>>>> commit 2a1d50fc12f6e604da834fbaea961d412aae6e85
>>>> Author:     Andrew Gierth <andrew@tao146.riddles.org.uk =
<mailto:andrew@tao146.riddles.org.uk>>
>>>> AuthorDate: 2023-12-24 12:04:21 +0000
>>>> Commit:     Konstantin Belousov <kib@FreeBSD.org =
<mailto:kib@FreeBSD.org>>
>>>> CommitDate: 2023-12-26 01:35:46 +0000
>>>>=20
>>>>   vfs_domount_update(): correct fsidcmp() usage
>>>>=20
>>>>   MFC after:      3 days
>>>> ---
>>>> sys/kern/vfs_mount.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>=20
>>>> 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 =3D EINVAL;
>>>> 			goto end;
>>>> 		}
>>>> -		if (fsidcmp(&fsid_up, &mp->mnt_stat.f_fsid) !=3D 0) {
>>>> +		if (fsidcmp(fsid_up, &mp->mnt_stat.f_fsid) !=3D 0) {
>>>> 			error =3D ENOENT;
>>>> 			goto end;
>>>> 		}
>>>=20
>>>=20
>>=20
>> 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 ;)

>=20
>>=20
>>>=20
>>> I guess we want to ensure ` typeof(a) =3D=3D typeof(b) =3D=3D fsid_t =
`, to prevent such kind of error in future.
>>>=20
>>> I see both gcc [1] and clang [2] have __builtin_types_compatible_p , =
so probably a good definition of fsidcmp should be
>>>=20
>>> ```
>>> #define TYPEASSERT(v, t) \
>>> 	_Static_assert(__builtin_types_compatible_p(typeof(v), t), =
"Requires type '" #t "'")
>>>=20
>>> #define fsidcmp(a, b) ({ \
>>> 	TYPEASSERT((a), fsid_t *); \
>>> 	TYPEASSERT((b), fsid_t *); \
>>> 	memcmp((a), (b), sizeof(fsid_t)); \
>>> })
>>> ```
>>>=20
>>> 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
>>>=20
>>=20
>>=20






Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?640284C5-ACE1-4D31-9BBB-AF181C0C7632>