On Fri, Jul 11, 2025 at 11:46:33AM -0400, John Baldwin wrote:
> On 7/8/25 17:49, Konstantin Belousov wrote:
> > The branch main has been updated by kib:
> > 
> > URL: 
> > https://cgit.FreeBSD.org/src/commit/?id=ba0d9b43e940077f4025e7e4e85d16c8d525db79
> > 
> > commit ba0d9b43e940077f4025e7e4e85d16c8d525db79
> > Author:     Konstantin Belousov <k...@freebsd.org>
> > AuthorDate: 2025-07-08 16:30:29 +0000
> > Commit:     Konstantin Belousov <k...@freebsd.org>
> > CommitDate: 2025-07-08 21:48:59 +0000
> > 
> >      kern_descrip.c: provide helpers to translate between fd flags namespace
> >      Reviewed by:    markj
> >      Sponsored by:   The FreeBSD Foundation
> >      Differential revision:  https://reviews.freebsd.org/D51206
> > ---
> >   sys/kern/kern_descrip.c | 110 
> > +++++++++++++++++++++++++++++++++++++++---------
> >   1 file changed, 90 insertions(+), 20 deletions(-)
> > 
> > diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
> > index 406236fc2723..2e1da2fdee29 100644
> > --- a/sys/kern/kern_descrip.c
> > +++ b/sys/kern/kern_descrip.c
> > @@ -480,6 +480,90 @@ kern_fcntl_freebsd(struct thread *td, int fd, int cmd, 
> > intptr_t arg)
> >     return (error);
> >   }
> > +struct flags_trans_elem {
> > +   u_int f;
> > +   u_int t;
> > +};
> > +
> > +static u_int
> > +flags_trans(const struct flags_trans_elem *ftes, int nitems, u_int 
> > from_flags)
> > +{
> > +   u_int res;
> > +   int i;
> > +
> > +   res = 0;
> > +   for (i = 0; i < nitems; i++) {
> > +           if ((from_flags & ftes[i].f) != 0)
> > +                   res |= ftes[i].t;
> > +   }
> > +   return (res);
> > +}
> > +
> > +static uint8_t
> > +fd_to_fde_flags(int fd_flags)
> > +{
> > +   static const struct flags_trans_elem fd_to_fde_flags_s[] = {
> > +           { .f = FD_CLOEXEC,              .t = UF_EXCLOSE },
> > +           { .f = FD_CLOFORK,              .t = UF_FOCLOSE },
> > +           { .f = FD_RESOLVE_BENEATH,      .t = UF_RESOLVE_BENEATH },
> > +   };
> > +
> > +   return (flags_trans(fd_to_fde_flags_s, nitems(fd_to_fde_flags_s),
> > +       fd_flags));
> > +}
> > +
> > +static int
> > +fde_to_fd_flags(uint8_t fde_flags)
> > +{
> > +   static const struct flags_trans_elem fde_to_fd_flags_s[] = {
> > +           { .f = UF_EXCLOSE,              .t = FD_CLOEXEC },
> > +           { .f = UF_FOCLOSE,              .t = FD_CLOFORK },
> > +           { .f = UF_RESOLVE_BENEATH,      .t = FD_RESOLVE_BENEATH },
> > +   };
> > +
> > +   return (flags_trans(fde_to_fd_flags_s, nitems(fde_to_fd_flags_s),
> > +       fde_flags));
> > +}
> > +
> > +static uint8_t
> > +fddup_to_fde_flags(int fddup_flags)
> > +{
> > +   static const struct flags_trans_elem fddup_to_fde_flags_s[] = {
> > +           { .f = FDDUP_FLAG_CLOEXEC,      .t = UF_EXCLOSE },
> > +           { .f = FDDUP_FLAG_CLOFORK,      .t = UF_FOCLOSE },
> > +   };
> > +
> > +   return (flags_trans(fddup_to_fde_flags_s, nitems(fddup_to_fde_flags_s),
> > +       fddup_flags));
> > +}
> > +
> > +static uint8_t
> > +close_range_to_fde_flags(int close_range_flags)
> > +{
> > +   static const struct flags_trans_elem close_range_to_fde_flags_s[] = {
> > +           { .f = CLOSE_RANGE_CLOEXEC,     .t = UF_EXCLOSE },
> > +           { .f = CLOSE_RANGE_CLOFORK,     .t = UF_FOCLOSE },
> > +   };
> > +
> > +   return (flags_trans(close_range_to_fde_flags_s,
> > +      nitems(close_range_to_fde_flags_s), close_range_flags));
> > +}
> > +
> > +static uint8_t
> > +open_to_fde_flags(int open_flags, bool sticky_orb)
> > +{
> > +   static const struct flags_trans_elem open_to_fde_flags_s[] = {
> > +           { .f = O_CLOEXEC,               .t = UF_EXCLOSE },
> > +           { .f = O_CLOFORK,               .t = UF_FOCLOSE },
> > +           { .f = O_RESOLVE_BENEATH,       .t = UF_RESOLVE_BENEATH },
> > +   };
> > +   _Static_assert(open_to_fde_flags_s[nitems(open_to_fde_flags_s) - 1].f ==
> > +       O_RESOLVE_BENEATH, "O_RESOLVE_BENEATH must be last, for 
> > sticky_orb");
> 
> This broke the GCC builds it seems.  GCC doesn't think that it can compute 
> this
> expression at compile time.
> 
> From https://ci.freebsd.org/job/FreeBSD-main-amd64-gcc14_build/1022/console:
> 
> 10:32:02 /workspace/src/sys/kern/kern_descrip.c: In function 
> 'open_to_fde_flags':
> 10:32:02 /workspace/src/sys/kern/kern_descrip.c:560:79: error: expression in 
> static =
> 10:32:02 assertion is not constant
> 10:32:02   560 |         
> _Static_assert(open_to_fde_flags_s[nitems(open_to_fde_flags=
> 10:32:02 _s) - 1].f =3D=3D
> 

I am unable to propose anything better than the patch below.  We have enough
#ifdef __clang__ in non-contrib sources already, so this should be not a
huge crime?

% rg -g '!contrib/*'  __clang__  | wc -l
     171

commit 96e6b15b0a1407af45ebe1ad38ee53627d09d16b
Author: Konstantin Belousov <k...@freebsd.org>
Date:   Fri Jul 11 21:57:32 2025 +0300

    kern_descrip.c: only allow complex expression in Static_assert() for clang
    
    gcc cannot compute the assert expression.
    
    Reported by:    jhb

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 93bdd41d1515..c4f5e586b0ed 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -557,8 +557,10 @@ open_to_fde_flags(int open_flags, bool sticky_orb)
                { .f = O_CLOFORK,               .t = UF_FOCLOSE },
                { .f = O_RESOLVE_BENEATH,       .t = UF_RESOLVE_BENEATH },
        };
+#ifdef __clang__
        _Static_assert(open_to_fde_flags_s[nitems(open_to_fde_flags_s) - 1].f ==
            O_RESOLVE_BENEATH, "O_RESOLVE_BENEATH must be last, for 
sticky_orb");
+#endif
 
        return (flags_trans(open_to_fde_flags_s, nitems(open_to_fde_flags_s) -
            (sticky_orb ? 0 : 1), open_flags));

Reply via email to