Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Dec 2023 11:29:50 +0800
From:      Zhenlei Huang <zlei@FreeBSD.org>
To:        Konstantin Belousov <kib@FreeBSD.org>
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:  <72190C27-0D45-4300-BB78-719F5EB510E9@FreeBSD.org>
In-Reply-To: <202312260136.3BQ1aOxq051977@gitrepo.freebsd.org>
References:  <202312260136.3BQ1aOxq051977@gitrepo.freebsd.org>

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

--Apple-Mail=_C781C9AB-69B4-4FE2-8EA1-71AB19AA400E
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii



> On Dec 26, 2023, at 9:36 AM, Konstantin Belousov <kib@FreeBSD.org> =
wrote:
>=20
> The branch main has been updated by kib:
>=20
> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D2a1d50fc12f6e604da834fbaea961d41=
2aae6e85
>=20
> commit 2a1d50fc12f6e604da834fbaea961d412aae6e85
> Author:     Andrew Gierth <andrew@tao146.riddles.org.uk>
> AuthorDate: 2023-12-24 12:04:21 +0000
> Commit:     Konstantin Belousov <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;
> 		}



I guess we want to ensure ` typeof(a) =3D=3D typeof(b) =3D=3D 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
2. https://clang.llvm.org/docs/LanguageExtensions.html

Best regards,
Zhenlei


--Apple-Mail=_C781C9AB-69B4-4FE2-8EA1-71AB19AA400E
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
	charset=us-ascii

<html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; =
charset=3Dus-ascii"></head><body style=3D"word-wrap: break-word; =
-webkit-nbsp-mode: space; line-break: after-white-space;" class=3D""><br =
class=3D""><div><br class=3D""><blockquote type=3D"cite" class=3D""><div =
class=3D"">On Dec 26, 2023, at 9:36 AM, Konstantin Belousov &lt;<a =
href=3D"mailto:kib@FreeBSD.org" class=3D"">kib@FreeBSD.org</a>&gt; =
wrote:</div><br class=3D"Apple-interchange-newline"><div class=3D""><div =
class=3D"">The branch main has been updated by kib:<br class=3D""><br =
class=3D"">URL: <a =
href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D2a1d50fc12f6e604da834fba=
ea961d412aae6e85" =
class=3D"">https://cgit.FreeBSD.org/src/commit/?id=3D2a1d50fc12f6e604da834=
fbaea961d412aae6e85</a><br class=3D""><br class=3D"">commit =
2a1d50fc12f6e604da834fbaea961d412aae6e85<br class=3D"">Author: =
&nbsp;&nbsp;&nbsp;&nbsp;Andrew Gierth &lt;<a =
href=3D"mailto:andrew@tao146.riddles.org.uk" =
class=3D"">andrew@tao146.riddles.org.uk</a>&gt;<br class=3D"">AuthorDate: =
2023-12-24 12:04:21 +0000<br class=3D"">Commit: =
&nbsp;&nbsp;&nbsp;&nbsp;Konstantin Belousov &lt;<a =
href=3D"mailto:kib@FreeBSD.org" class=3D"">kib@FreeBSD.org</a>&gt;<br =
class=3D"">CommitDate: 2023-12-26 01:35:46 +0000<br class=3D""><br =
class=3D""> &nbsp;&nbsp;&nbsp;vfs_domount_update(): correct fsidcmp() =
usage<br class=3D""><br class=3D""> &nbsp;&nbsp;&nbsp;MFC after: =
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;3 days<br class=3D"">---<br class=3D""> =
sys/kern/vfs_mount.c | 2 +-<br class=3D""> 1 file changed, 1 =
insertion(+), 1 deletion(-)<br class=3D""><br class=3D"">diff --git =
a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c<br class=3D"">index =
8e54c832e9f1..331e4887c200 100644<br class=3D"">--- =
a/sys/kern/vfs_mount.c<br class=3D"">+++ b/sys/kern/vfs_mount.c<br =
class=3D"">@@ -1388,7 +1388,7 @@ vfs_domount_update(<br class=3D""> =
<span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span>error =3D EINVAL;<br class=3D""> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span>goto end;<br class=3D""> <span =
class=3D"Apple-tab-span" style=3D"white-space:pre">	</span><span =
class=3D"Apple-tab-span" style=3D"white-space:pre">	</span>}<br =
class=3D"">-<span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span>if (fsidcmp(&amp;fsid_up, &amp;mp-&gt;mnt_stat.f_fsid) !=3D 0) =
{<br class=3D"">+<span class=3D"Apple-tab-span" style=3D"white-space:pre">=
	</span><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span>if (fsidcmp(fsid_up, &amp;mp-&gt;mnt_stat.f_fsid) !=3D 0) =
{</div></div></blockquote><blockquote type=3D"cite" class=3D""><div =
class=3D""><div class=3D""> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span>error =3D ENOENT;<br class=3D""> =
<span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span>goto end;<br class=3D""> <span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span><span class=3D"Apple-tab-span" =
style=3D"white-space:pre">	</span>}<br =
class=3D""></div></div></blockquote></div><div class=3D""><br =
class=3D""></div><div class=3D""><br class=3D""></div><div class=3D"">I =
guess we want to ensure ` typeof(a) =3D=3D&nbsp;typeof(b) =3D=3D&nbsp;<spa=
n style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" =
class=3D"">fsid_t `, to prevent such kind of error in =
future.</span></div><div class=3D""><br class=3D""></div><div =
class=3D""><font color=3D"#000000" class=3D""><span style=3D"caret-color: =
rgb(0, 0, 0);" class=3D"">I see both gcc [1] and clang [2] =
have&nbsp;__builtin_types_compatible_p , so probably a good definition =
of&nbsp;</span></font><span style=3D"caret-color: rgb(0, 0, 0); color: =
rgb(0, 0, 0);" class=3D"">fsidcmp should be</span></div><div =
class=3D""><span style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, =
0);" class=3D""><br class=3D""></span></div><div class=3D""><span =
style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" =
class=3D"">```</span></div><div class=3D""><font color=3D"#000000" =
class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" class=3D"">#define =
TYPEASSERT(v, t) \</span></font></div><div class=3D""><font =
color=3D"#000000" class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" =
class=3D""><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span>_Static_assert(__builtin_types_compatible_p(typeof(v), t), =
"Requires type '" #t "'")</span></font></div><div class=3D""><span =
style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class=3D""><br =
class=3D""></span></div><div class=3D""><font color=3D"#000000" =
class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" class=3D"">#define =
fsidcmp(a, b) ({ \</span></font></div><div class=3D""><font =
color=3D"#000000" class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" =
class=3D""><span class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span></span></font><span style=3D"caret-color: rgb(0, 0, 0); color: =
rgb(0, 0, 0);" class=3D"">TYPEASSERT((a),&nbsp;</span><span =
style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" =
class=3D"">fsid_t *); \</span></div><div class=3D""><span =
style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" class=3D""><span=
 class=3D"Apple-tab-span" style=3D"white-space:pre">	=
</span>TYPEASSERT((b),&nbsp;</span><span style=3D"caret-color: rgb(0, 0, =
0); color: rgb(0, 0, 0);" class=3D"">fsid_t *); \</span></div><div =
class=3D""><span class=3D"Apple-tab-span" style=3D"caret-color: rgb(0, =
0, 0); color: rgb(0, 0, 0); white-space: pre;">	</span><span =
style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);" =
class=3D"">memcmp((a), (b), sizeof(fsid_t)); \</span></div><div =
class=3D""><div class=3D""><font color=3D"#000000" class=3D""><span =
style=3D"caret-color: rgb(0, 0, 0);" class=3D"">})</span></font></div><div=
 class=3D""><span style=3D"caret-color: rgb(0, 0, 0); color: rgb(0, 0, =
0);" class=3D"">```</span></div><div class=3D""><font color=3D"#000000" =
class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" class=3D""><br =
class=3D""></span></font></div><div class=3D""><font color=3D"#000000" =
class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" class=3D""><div =
class=3D"">1. <a =
href=3D"https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html" =
class=3D"">https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html</a></div=
><div class=3D"">2. <a =
href=3D"https://clang.llvm.org/docs/LanguageExtensions.html" =
class=3D"">https://clang.llvm.org/docs/LanguageExtensions.html</a></div></=
span></font></div><br class=3D""><div class=3D"">
<div>Best regards,</div><div>Zhenlei</div>

</div>
<br class=3D""></div></body></html>=

--Apple-Mail=_C781C9AB-69B4-4FE2-8EA1-71AB19AA400E--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?72190C27-0D45-4300-BB78-719F5EB510E9>