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: <20181222172859.6d3b84ae@archlinux>
Date:   Sat, 22 Dec 2018 17:28:59 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Amir Mahdi Ghorbanian <indigoomega021@...il.com>
Cc:     lars@...afoo.de, Michael.Hennerich@...log.com, 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 Fri, 21 Dec 2018 15:26:26 -0800
Amir Mahdi Ghorbanian <indigoomega021@...il.com> wrote:

> Replaced bool in struct with unsigned int bitfield to conserve space and
> more clearly define size of varibales
> 
> Signed-off-by: Amir Mahdi Ghorbanian <indigoomega021@...il.com>
Hi Amir,

I'm a bit in two minds on this one.  It's not a size critical structure
and the advantage of bools is they make it clear the thing really is
true or false.

Another element here is that this is a platform data structure and unless
we have users in the kernel tree, the chances that we'll maintain it's
existence at all as we look to move this driver out of staging is very
small!

So slightly marginal advantage to the change on a structure that I certainly
hope is shortly going away!

Sorry, but I'm not convinced and won't be applying it.

If you want to work on this driver though that would be great and I'm happy
to do a review of it to highlight what other elements need cleaning up.
Just say on the list and either I'll take a look or one of our other
reviewers will.

Thanks,

Jonathan

> ---
>  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_ */