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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 15 Nov 2017 18:35:51 +0900
From:   James Bottomley <James.Bottomley@...senPartnership.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Hannes Reinecke <hare@...e.de>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-scsi <linux-scsi@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [GIT PULL] first round of SCSI updates for the 4.14+ merge
 window

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@...senpartnership.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