linux-kernel - Re: [PATCH] Staging: iio: ad7192: replaced bool in struct

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: <8E720BF2-7AFC-4FFA-8FA2-5A403FD5DD1D@konsulko.com>
Date:   Wed, 2 Jan 2019 13:25:27 -0800
From:   Matt Ranostay <matt.ranostay@...sulko.com>
To:     Himanshu Jha <himanshujha199640@...il.com>
Cc:     Amir Mahdi Ghorbanian <indigoomega021@...il.com>, lars@...afoo.de,
        Michael.Hennerich@...log.com, jic23@...nel.org, knaack.h@....de,
        pmeerw@...erw.net, gregkh@...uxfoundation.org,
        linux-iio@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Staging: iio: ad7192: replaced bool in struct


> On Dec 24, 2018, at 01:58, Himanshu Jha <himanshujha199640@...il.com> wrote:
> 
>> On Fri, Dec 21, 2018 at 03:26:26PM -0800, Amir Mahdi Ghorbanian wrote:
>> Replaced bool in struct with unsigned int bitfield to conserve space and
>> more clearly define size of varibales

Important thing to note is depending on padding, alignment, and position of field it probably won’t save any space.

Also for internal unpacked structures it really makes little sense to save a few bytes of data. Don’t read into that packed structures are good.. they usually aren’t :)

- Matt
>> 
>> Signed-off-by: Amir Mahdi Ghorbanian <indigoomega021@...il.com>
>> ---
> 
> There was some discussion on this at Outreachy list:
> https://groups.google.com/d/msg/outreachy-kernel/xpQAl-Gn8HA/yqcQRG_qBgAJ
> 
> I think unless you post some statistics about 'conserving' space, 
> it is unlikely that maintainers will apply it.
> 
> This idea was originally given by Linus and that thread of discussion 
> is worth reading too.
> 
>> drivers/staging/iio/adc/ad7192.h | 14 +++++++-------
>> 1 file changed, 7 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/staging/iio/adc/ad7192.h b/drivers/staging/iio/adc/ad7192.h
>> index 7433a43..7d3e62f 100644
>> --- a/drivers/staging/iio/adc/ad7192.h
>> +++ b/drivers/staging/iio/adc/ad7192.h
>> @@ -35,13 +35,13 @@ struct ad7192_platform_data {
>>    u16        vref_mv;
>>    u8        clock_source_sel;
>>    u32        ext_clk_hz;
>> -    bool        refin2_en;
>> -    bool        rej60_en;
>> -    bool        sinc3_en;
>> -    bool        chop_en;
>> -    bool        buf_en;
>> -    bool        unipolar_en;
>> -    bool        burnout_curr_en;
>> +    unsigned int    refin2_en : 1;
>> +    unsigned int    rej60_en : 1;
>> +    unsigned int    sinc3_en : 1;
>> +    unsigned int    chop_en : 1;
>> +    unsigned int    buf_en : 1;
>> +    unsigned int    unipolar_en : 1;
>> +    unsigned int    burnout_curr_en : 1;
>> };
>> 
>> #endif /* IIO_ADC_AD7192_H_ */
>> -- 
>> 2.7.4
>> 
> 
> Goodluck!
> 
> -- 
> Himanshu Jha
> Undergraduate Student
> Department of Electronics & Communication
> Guru Tegh Bahadur Institute of Technology