netdev - rfc: bool structure members (was Re: [PATCH V3] net/mlx4: Get rid of page operation after dma_alloc

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] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Dec 2018 18:25:05 -0800
From:   Joe Perches <joe@...ches.com>
To:     Bart Van Assche <bvanassche@....org>
Cc:     Stephen Warren <swarren@...dotorg.org>,
        Tariq Toukan <tariqt@...lanox.com>, xavier.huwei@...wei.com,
        netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
        Doug Ledford <dledford@...hat.com>,
        Stephen Warren <swarren@...dia.com>,
        Christoph Hellwig <hch@....de>, Jason Gunthorpe <jgg@...pe.ca>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: rfc: bool structure members (was Re: [PATCH V3] net/mlx4: Get rid
 of page operation after dma_alloc_coherent)

On Thu, 2018-12-20 at 09:49 -0800, Bart Van Assche wrote:
> On Thu, 2018-12-20 at 18:44 +0100, Christoph Hellwig wrote:
> > On Thu, Dec 20, 2018 at 10:43:18AM -0700, Jason Gunthorpe wrote:
> > > >   - chunk->coherent is an int not a bool since checkpatch complains about
> > > >     using bool in structs; see https://lkml.org/lkml/2017/11/21/384.
> > > 
> > > :( bool is much more readable and there is no performance concern in
> > > this struct. I think checkpatch is overzealous here.
> > 
> > Yes.  Nevermind that this for bool vs bitfield.  A int is worse in
> > every respect in the criteria used in that mail.
> 
> (+Joe Perches)
> 
> Hi Joe,

Hi all.

> This is the second time that I see that the checkpatch complaint about using
> bool in a structure leads kernel contributors to a bad decision. Please consider
> removing that warning from checkpatch.

I agree it's not a very good message nor is bool use
of structure members a real problem except in very
few cases.

I think the message could either be restated and bool
members described as OK for unshared memory structures.

Right now this is the test:

# check for bool use in .h files
		if ($realfile =~ /\.h$/ &&
		    $sline =~ /^.\s+bool\s*$Ident\s*(?::\s*d+\s*)?;/) {
			CHK("BOOL_MEMBER",
			    "Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384\n" . $herecurr);
		}

What do you believe would be better message wording
or a perhaps an improved test?

btw: this can emit false positives for .h files with
static inline functions that declare bool automatics.

Linus' original email (https://lkml.org/lkml/2017/11/21/384) was:
---------------------------------------------------------------------
As others have already said, please don't use "bool" in structures at
all. It's an incredible waste of space, but it's also not entirely
clear what the type size even is. Usually a byte, but I don't think
there is any guarantee of it at all.

Use "bool" mainly as a return type from functions that return
true/false, and maybe as local variables in functions.

Maybe using single-bit bitfield with "bool" as the base type might
work, but the normal thing we tend to do is to just make sure the base
type is unsigned. It's a tiny bit more typing, but it's very
unambiguous what is going on, and you can specify the base type as you
wish and as it makes sense for packing etc.