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.

> 
> > 
> > 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
> > 
> 
> 

Reply via email to