Re: [GIT PULL] first round of SCSI updates for the 4.14+ merge window

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Tue, 2017-11-14 at 16:33 -0800, Linus Torvalds wrote:
> On Tue, Nov 14, 2017 at 8:36 AM, James Bottomley
> <James.Bottomley@hansenpartnership.com> 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?

It will, boss; I'll make sure of it.

James