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