Date: Tue, 26 Dec 2023 11:31:38 +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: <78061056-76D8-457C-A522-8CE62B481317@FreeBSD.org> In-Reply-To: <72190C27-0D45-4300-BB78-719F5EB510E9@FreeBSD.org> References: <202312260136.3BQ1aOxq051977@gitrepo.freebsd.org> <72190C27-0D45-4300-BB78-719F5EB510E9@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_B4F4564D-8E62-412A-A497-93B918D5E772 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > 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 The fsidcmp is currently defined as ``` #define fsidcmp(a, b) memcmp((a), (b), sizeof(fsid_t)) ``` >=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 --Apple-Mail=_B4F4564D-8E62-412A-A497-93B918D5E772 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 11:29 AM, Zhenlei Huang <<a = href=3D"mailto:zlei@FreeBSD.org" class=3D"">zlei@FreeBSD.org</a>> = wrote:</div><br class=3D"Apple-interchange-newline"><div class=3D""><meta = http-equiv=3D"Content-Type" content=3D"text/html; charset=3Dus-ascii" = class=3D""><div style=3D"word-wrap: break-word; -webkit-nbsp-mode: = space; line-break: after-white-space;" class=3D""><br class=3D""><div = class=3D""><br class=3D""><blockquote type=3D"cite" class=3D""><div = class=3D"">On Dec 26, 2023, at 9:36 AM, Konstantin Belousov <<a = href=3D"mailto:kib@FreeBSD.org" class=3D"">kib@FreeBSD.org</a>> = 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: = Andrew Gierth <<a = href=3D"mailto:andrew@tao146.riddles.org.uk" = class=3D"">andrew@tao146.riddles.org.uk</a>><br class=3D"">AuthorDate: = 2023-12-24 12:04:21 +0000<br class=3D"">Commit: = Konstantin Belousov <<a = href=3D"mailto:kib@FreeBSD.org" class=3D"">kib@FreeBSD.org</a>><br = class=3D"">CommitDate: 2023-12-26 01:35:46 +0000<br class=3D""><br = class=3D""> vfs_domount_update(): correct fsidcmp() = usage<br class=3D""><br class=3D""> MFC after: = 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(&fsid_up, &mp->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, &mp->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></div></blockquote><div><br = class=3D""></div><div>The fsidcmp is currently defined = as</div><div>```</div><div>#define fsidcmp(a, b) memcmp((a), (b), = sizeof(fsid_t))</div><div>```</div><br class=3D""><blockquote = type=3D"cite" class=3D""><div class=3D""><div style=3D"word-wrap: = break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" = class=3D""><div class=3D""><br class=3D""></div><div class=3D"">I guess = we want to ensure ` typeof(a) =3D=3D typeof(b) =3D=3D <span = style=3D"caret-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 class=3D""><span = style=3D"caret-color: rgb(0, 0, 0);" class=3D"">I see both gcc [1] and = clang [2] have __builtin_types_compatible_p , so probably a good = definition of </span></font><span style=3D"caret-color: rgb(0, 0, = 0);" class=3D"">fsidcmp should be</span></div><div class=3D""><span = style=3D"caret-color: rgb(0, 0, 0);" class=3D""><br = class=3D""></span></div><div class=3D""><span style=3D"caret-color: = rgb(0, 0, 0);" class=3D"">```</span></div><div class=3D""><font = class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" class=3D"">#define = TYPEASSERT(v, t) \</span></font></div><div class=3D""><font = 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);" class=3D""><br = class=3D""></span></div><div class=3D""><font class=3D""><span = style=3D"caret-color: rgb(0, 0, 0);" class=3D"">#define fsidcmp(a, b) ({ = \</span></font></div><div class=3D""><font 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);" = class=3D"">TYPEASSERT((a), </span><span style=3D"caret-color: = rgb(0, 0, 0);" class=3D"">fsid_t *); \</span></div><div 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>TYPEASSERT((b), </span><span style=3D"caret-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); = white-space: pre;"> </span><span style=3D"caret-color: rgb(0, 0, = 0);" class=3D"">memcmp((a), (b), sizeof(fsid_t)); \</span></div><div = class=3D""><div class=3D""><font 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);" class=3D"">```</span></div><div = class=3D""><font class=3D""><span style=3D"caret-color: rgb(0, 0, 0);" = class=3D""><br class=3D""></span></font></div><div class=3D""><font = 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 class=3D"">Best regards,</div><div class=3D"">Zhenlei</div> </div> <br class=3D""></div></div></div></blockquote></div><br class=3D""><div = class=3D""> <div><br class=3D""></div></div></body></html>= --Apple-Mail=_B4F4564D-8E62-412A-A497-93B918D5E772--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?78061056-76D8-457C-A522-8CE62B481317>