On Tue, Nov 14, 2017 at 8:36 AM, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> Hannes Reinecke (14):
> scsi: scsi_devinfo: Reformat blacklist flags
Ugh, that's just really ugly, but it's also wrong.
Just having long lines would probably have been much preferable, and
would mean that the commit that explains the bit would show up when
you grep for the bit.
Having a small helper macro like
#define BLIST_n(x) ((__force __u32 __bitwise)(1 << (n)))
woiuld also likely have made it more legible.
But that only takes care of the ugliness and the greppability.
It's not right for sparse even _with_ those changes.
Why? Because "__bitwise" actually creates a new type. So what those
BLIST defines should do is to use a special type something like
typedef unsigned int __bitwise blist_flags_t;
and now you have _one_ type thanks to that typedef, that is different
from all the other bitwise types. Then you force all the constants and
the field that implements to have that type, and you have type-safety:
you can use those constants together, and you can assign the result to
the blist flags, but you can't mix it with other __bitwise types.
That's why things like this work:
typedef __u16 __bitwise __le16;
typedef __u16 __bitwise __be16;
where __le16 and __be16 are actually different types, even though
their underlying _storage_ is the same (a 16-bit unsigned).
Anyway, I've pulled, because clearly this only matters for sparse, but
I would hope that this gets fixed up, ok?
Linus
