On Fri, Sep 07, 2018 at 05:29:45PM -0700, Linus Torvalds wrote:
> On Fri, Sep 7, 2018 at 5:01 PM Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> >
> > In the kernel, some enums are defined with such bitwise types
> > as initializers; the goal being to have slightly more strict enums.
> > While the semantic of such enums is not very clear, using a mix
> > of bitwise and not-bitwise initializers completely defeats the
> > desired stricter typing.
>
> Hmm.
>
> We consider 0 to be special, in that it matches all bitwise types.
True (but '0 matches all bitwise types' only means that a plain 0 can
be assigned to a variable of some bitwise type, right?).
> So
>
> typedef long long __bitwise bits;
>
> enum a {
> one = 0,
> two = (__force bits) 1,
> };
>
> feels like it should work (and result in everything being of type
> "bits"), but looking at this patch I suspect it results in a warning.
Yes, with the patch we now have a warning 'mixed bitwiseness'.
It's more or less the intent but the motivation was to catch
the reversed situation like in drivers/net/wireless/intel/iwlwifi/fw/file.h
enum iwl_ucode_tlv_api {
/* API Set 0 */
IWL_UCODE_TLV_API_FRAGMENTED_SCAN = (__force iwl_ucode_tlv_api_t)8,
IWL_UCODE_TLV_API_WIFI_MCC_UPDATE = (__force iwl_ucode_tlv_api_t)9,
...
IWL_UCODE_TLV_API_ADAPTIVE_DWELL_V2 = (__force iwl_ucode_tlv_api_t)42,
NUM_IWL_UCODE_TLV_API
#ifdef __CHECKER__
/* sparse says it cannot increment the previous enum member */
= 128
#endif
};
The core of the problem here is to find some base/underlying type
for the enum. Without this series, the underlying type is 'bad_ctype',
without any warnings. Most things seems to work correctly but it's
only an apparence.
In the example you're giving, what should be the base type of the enum?
After 'one' is processed, nothing is known yet about 'two' being
a bitwise type and so '0' can't be bitwise-magical.
I think that things will be much simpler and clearer (semantically and
the code) when we will be able to write:
enum __bitwise a {
one = 0;
two = ...
};
but currently your example can only be handled if the base type is selected
in two passes. Is it worth?
-- Luc
