linux-kernel - Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in structures"

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]

Message-ID: <79cc1057-bd62-02bb-a8e4-a440aa469109@ideasonboard.com>
Date:   Fri, 18 Nov 2022 09:28:54 +0530
From:   Umang Jain <umang.jain@...asonboard.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kieran Bingham <kieran.bingham@...asonboard.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Dan Carpenter <error27@...il.com>,
        Dave Stevenson <dave.stevenson@...pberrypi.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Mauro Carvalho Chehab <mchehab+samsung@...nel.org>,
        Ray Jui <rjui@...adcom.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-rpi-kernel@...ts.infradead.org, linux-staging@...ts.linux.dev
Subject: Re: [PATCH 1/3] Revert "staging: mmal-vchiq: Avoid use of bool in
 structures"

Hi,

On 11/18/22 5:42 AM, Andrew Lunn wrote:
>>>>>>     struct vchiq_mmal_port {
>>>>>> -       u32 enabled:1;
>>>>>> +       bool enabled:1;
>>>>> Is this a direct revert with 'git revert' ?
>>>> No. It had conflicts plus I added the ':1' initialization to keep the logic
>>>> same (in case 'enabled' gets used directly). Similar pattern come up with:
>>>>       ($) git grep 'bool' -- '*.[h]' | grep '\:1'
>>>>
>>>> So it shouldn't be an issue.
>>> Please don't do that "bool foo:1" makes no sense.  Drop the ":1"
>>> please.
>> It won't affect this patch but if you take a look at 2/3 - you'll see a bool
>> flag 'in_use' that needs to be initialized (as it's getting used directly).
>>
>> I can move the initialization part in the function (_init() or something)
>> and drop the ":1" as you mentioned. That's  fine as well but I do find
>> patterns of 'bool foo:1' in the codebase so I assumed it would be safe to
>> use.
> Does :1 really initialise the variable? In "u32 enabled:1" it means
> this is a 1 bit wide bit field. It seems odd that bool is somehow
> special and :1 means something else.


Yup you are correct - seems I mis-read :1 as initialization

>
> 	Andrew