linux-kernel - Re: [greybus-dev] [PATCH] staging: greybus: fixed the coding style, labels should not be indented.

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: <10ad30e2-c906-b210-bf0e-5e20b6de1993@linaro.org>
Date:   Thu, 3 Jun 2021 16:55:04 -0500
From:   Alex Elder <elder@...aro.org>
To:     David Laight <David.Laight@...LAB.COM>,
        'Manikishan Ghantasala' <manikishanghantasala@...il.com>,
        Alex Elder <elder@...e.org>
Cc:     Alex Elder <elder@...nel.org>,
        "greybus-dev@...ts.linaro.org" <greybus-dev@...ts.linaro.org>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        Johan Hovold <johan@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [greybus-dev] [PATCH] staging: greybus: fixed the coding style,
 labels should not be indented.

On 6/3/21 4:48 PM, David Laight wrote:
> From: Alex Elder
>> Sent: 03 June 2021 22:46
>>
>> On 6/3/21 4:22 PM, David Laight wrote:
> ...
>>>>>> --- a/drivers/staging/greybus/gpio.c
>>>>>> +++ b/drivers/staging/greybus/gpio.c
>>>>>> @@ -20,9 +20,9 @@
>>>>>>     struct gb_gpio_line {
>>>>>>         /* The following has to be an array of line_max entries */
>>>>>>         /* --> make them just a flags field */
>>>>>> -     u8                      active:    1,
>>>>>> -                             direction: 1,   /* 0 = output, 1 = input */
>>>>>> -                             value:     1;   /* 0 = low, 1 = high */
>>>>>> +     u8                      active:1,
>>>>>> +                             direction:1,    /* 0 = output, 1 = input */
>>>>>> +                             value:1;        /* 0 = low, 1 = high */
>>>
>>> Why are you even using bitfields at all?
>>> If you cared about the structure size you'd not have a byte-size pad here.
>>
>> Apparently I committed this, and it was part of the very first
>> Greybus drivers...
>>
>> These would be better defined as Booleans; there are others in
>> the same structure after all.  That would have avoided the
>> checkpatch problem in the first place.
> 
> Using 'u8' can be sensible.
> Boolean will be 32bit.

Not necessarily, sizeof(bool) is implementation defined.
And I thought you didn't think the size of the structure
was very important...

In any case, I'm open to changing the type of these fields,
and my preference would be bool rather than u8, because it
is completely clear what it represents.

					-Alex

> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>