lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZMdu1YqUI7VIEq1y@alley>
Date:   Mon, 31 Jul 2023 10:20:37 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Dave Airlie <airlied@...il.com>, Arnd Bergmann <arnd@...db.de>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: arm32 build warnings in workqueue.c

On Fri 2023-06-23 09:47:56, Tejun Heo wrote:
> Hello,
> 
> On Fri, Jun 23, 2023 at 11:24:16AM -0700, Linus Torvalds wrote:
> ...
> >   enum {
> >         [..]
> >         WORK_STRUCT_FLAG_BITS   = WORK_STRUCT_COLOR_SHIFT +
> >                 WORK_STRUCT_COLOR_BITS,
> >         [..]
> >         WORK_STRUCT_FLAG_MASK   = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
> >         WORK_STRUCT_WQ_DATA_MASK = ~WORK_STRUCT_FLAG_MASK,
> >   }
> > 
> > Anyway, that code absolutely has to be fixed. Using enums for types is
> > wrong, wrong, wrong. You should consider an enum to be a weakly typed
> > integer expression with some specific advantages (the automatic
> > incrementing at definition time, and the compiler being able to limit
> > ranges in switch() statements and maybe doing limited type warnings
> > elsewhere)
> 
> The only benefit I care about is it being visible in the type system and
> thus in debug info whether that's the usual one or BTF. This makes a huge
> difference in tracing, monitoring and debugging. For one off's, it's fine to
> track down the define values. For something more sophisticated (e.g.
> non-trivial drgn, bcc and other BPF programs), the macro constants are a
> source of sadness and a really dumb and insidious one.

Would it help to use const variables?

> > +/* Convenience constants - of type 'unsigned long', not 'enum'! */
> > +#define WORK_OFFQ_CANCELING	(1ul << __WORK_OFFQ_CANCELING)

static const WORK_OFFQ_CANCELING = 1ul << __WORK_OFFQ_CANCELING;

It is kind of nasty as well. But it defines the right type and should
be visible in debuginfo. Well, it is usable only for local variables.

Best Regards,
Petr

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ