Re: enabling -Werror=assign-enum for kernel

On 8/7/26 18:39, Gleb Smirnoff wrote:
>    Hi,
> 
> modern clang has a nice feature: it allows to catch enum misuse.  For example:
> 
> enum foo { A = 1, B } var;
> var = 1;	/* success */
> var = A;	/* success */
> var = B;	/* success */
> var = 3;	/* fail */
> 
> What about enums that are used as flags? For that it has
> __attribute__((flag_enum)).  This will allow using combination of values and
> clearing them:
> 
> var = A | B;	/* success */
> var &= ~A;	/* success */
> var = 3;	/* fail */

Note that C++ doesn't allow this at the language level.  When I converted ctld to
C++ I had to adjust some enums to be simple #define's instead in the ctl headers.
  
> This seems like a nice enforcement of a good code.  I already found a few
> hamrless bugs with it and one in e1000 that could be a real bug.  It also
> highlights some sketchy code that better be refactored.
> 
> I have a branch where LINT is compilable with -Werror=assign-enum:
> 
> https://github.com/freebsd/freebsd-src/compare/main...glebius:FreeBSD:Werror%3Dassign-enum
> 
> There are basically three parts that require work:
> 
> 1) The most common violators are SYSINIT(9)s that use (SI_SUB_FOO + 1) as
> argument.  I have posted a pack of reviews to cover that.

So I think this is probably sensible for SI_SUB_* to define new values.  I
worry about SI_ORDER_*.  Possibly we should use plain #define's for SI_ORDER_*
instead of an enum if we aren't already.  I wonder if you ran into any cases
where we use an SI_ORDER_* value that is an expression?

> 2) hwpmc has enum pmc_event that is generated by preprocessor and its use is
> sketchy,    thus files that utilize HWPMC_HOOKS are temporarily excluded from
> the enforcements.
> 
> 3) I was too tired to look at OFED, thus just disabled it in ${OFED_C} in
> kern.mk

Also, we should avoid gratuitous diffs to OFED since it is really contrib code,
so your approach here is fine.  OTOH, note that other Linuxy bits like the wifi
drivers and drm drivers might also need to opt out of this via CFLAGS.

-- 
John Baldwin