linux-kernel - Re: [PATCH v6 5/8] iio: dac: ad3552r: changes to use FIELD

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: <aae0d87ff9506198e0d9bf3abee6cd460c655ea0.camel@gmail.com>
Date: Tue, 15 Oct 2024 08:17:39 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, Angelo Dureghello
 <adureghello@...libre.com>, Nuno Sá
 <nuno.sa@...log.com>,  Lars-Peter Clausen <lars@...afoo.de>, Michael
 Hennerich <Michael.Hennerich@...log.com>,  Jonathan Cameron
 <jic23@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Olivier Moysan
 <olivier.moysan@...s.st.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v6 5/8] iio: dac: ad3552r: changes to use FIELD_PREP

On Mon, 2024-10-14 at 16:14 -0500, David Lechner wrote:
> On 10/14/24 5:08 AM, Angelo Dureghello wrote:
> > From: Angelo Dureghello <adureghello@...libre.com>
> > 
> > Changes to use FIELD_PREP, so that driver-specific ad3552r_field_prep
> > is removed. Variables (arrays) that was used to call ad3552r_field_prep
> > are removed too.
> > 
> > Signed-off-by: Angelo Dureghello <adureghello@...libre.com>
> > ---
> 
> Found one likely bug. The rest are suggestions to keep the static
> analyzers happy.
> 
> 				\
> > @@ -510,8 +416,14 @@ static int ad3552r_write_raw(struct iio_dev *indio_dev,
> >  					val);
> >  		break;
> >  	case IIO_CHAN_INFO_ENABLE:
> > -		err = ad3552r_set_ch_value(dac, AD3552R_CH_DAC_POWERDOWN,
> > -					   chan->channel, !val);
> > +		if (chan->channel == 0)
> > +			val = FIELD_PREP(AD3552R_MASK_CH_DAC_POWERDOWN(0),
> > !val);
> > +		else
> > +			val = FIELD_PREP(AD3552R_MASK_CH_DAC_POWERDOWN(1),
> > !val);
> 
> In the past, I've had bots (Sparse, IIRC) complain about using !val
> with FIELD_PREP. Alternative is to write it as val ? 1 : 0.
> 

Hmm, I'm fairly sure I also suffered from that warning. AFAICT, there's nothing wrong
with the code so I would not make it less readable just to keep the tool happy (it
seems to me that the tool is the one that needs to make this right). But this is just
me - yeah, not a fan of the ternary operator :)

Anyways, no strong feelings so if you go with the above, I won't really complain...
just my 2 cents.

- Nuno Sá 
>