Re: [axboe-block:for-6.10/io_uring 42/42] io_uring/register.c:175:24: warning: arithmetic between different enumeration types ('enum io_uring_register_restriction_op' and 'enum io_uring_register_op')

[Nathan Chancellor <nathan@kernel.org>]

On Fri, Mar 29, 2024 at 05:15:44PM -0600, Jens Axboe wrote:
> On 3/29/24 4:04 PM, Gabriel Krisman Bertazi wrote:
> > kernel test robot <lkp@intel.com> writes:
> > 
> > [+ io_uring list ]
> > 
> >>>> io_uring/register.c:175:24: warning: arithmetic between different
> >> enumeration types ('enum io_uring_register_restriction_op' and 'enum
> >> io_uring_register_op') [-Wenum-enum-conversion]
> >>      175 |         if (!arg || nr_args > IORING_MAX_RESTRICTIONS)
> >>          |                               ^~~~~~~~~~~~~~~~~~~~~~~
> >>    io_uring/register.c:31:58: note: expanded from macro 'IORING_MAX_RESTRICTIONS'
> >>       31 | #define IORING_MAX_RESTRICTIONS (IORING_RESTRICTION_LAST + \
> >>          |                                  ~~~~~~~~~~~~~~~~~~~~~~~ ^
> >>       32 |                                  IORING_REGISTER_LAST + IORING_OP_LAST)
> >>          |                                  ~~~~~~~~~~~~~~~~~~~~
> >>    14 warnings generated.
> > 
> > hm.
> > 
> > Do we want to fix?  The arithmetic is safe here.  I actually tried
> > triggering the warning with gcc, but even with -Wenum-conversion in
> > gcc-12 (which is in -Wextra and we don't use in the kernel build), I
> > couldn't do it.  only llvm catches this.
> > 
> > can we explicit cast to int to silent it?
> 
> I don't think we care, there are others like it in the kernel already.
> Plus you need some magic warning incantation to hit it, clang by default
> is not going to be enough.

The magic incantion would be W=1 after commit 75b5ab134bb5 ("kbuild:
Move -Wenum-{compare-conditional,enum-conversion} into W=1") but yes,
there are plenty of other instances of this warning in the kernel and if
the arithmetic is safe, ignoring the warning is perfectly fine in my
opinion. The primary reason it is in W=1 rather than off by default is
to see how many current instances may be bugs and see what new instances
may pop up, as they may be unsafe conversions. We will have to come up
with some way to annotate safe conversions if we want to turn this on by
default (which is still being determined at this point). You do also
have to have a tip of tree Clang to see this at the moment as well.

Cheers,
Nathan