linux-kernel - Re: arm32 build warnings in workqueue.c

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]

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