linux-kernel - Re: [PATCH v3 3/6] iio: adc: ad7173: refactor ain and vref selection

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: <20240601194027.67c97d76@jic23-huawei>
Date: Sat, 1 Jun 2024 19:40:27 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Nuno Sá <noname.nuno@...il.com>
Cc: "Ceclan, Dumitru" <mitrutzceclan@...il.com>, dumitru.ceclan@...log.com,
 Lars-Peter Clausen <lars@...afoo.de>, Michael Hennerich
 <Michael.Hennerich@...log.com>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley
 <conor+dt@...nel.org>, David Lechner <dlechner@...libre.com>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/6] iio: adc: ad7173: refactor ain and vref
 selection

On Fri, 31 May 2024 09:10:43 +0200
Nuno Sá <noname.nuno@...il.com> wrote:

> On Thu, 2024-05-30 at 17:45 +0300, Ceclan, Dumitru wrote:
> > On 29/05/2024 15:49, Nuno Sá wrote:  
> > > On Wed, 2024-05-29 at 14:27 +0200, Nuno Sá wrote:  
> > > > On Mon, 2024-05-27 at 20:02 +0300, Dumitru Ceclan via B4 Relay wrote:  
> > > > > From: Dumitru Ceclan <dumitru.ceclan@...log.com>  
> > 
> > ...
> >   
> > > > > +static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
> > > > > +					      unsigned int ain[2])  
> > > 
> > > Pass the pointer and size of it... Also, it should be made 'const'
> > >   
> > 
> > I'm learning here: what is the purpose of passing the size of it?
> > This is a specific case where the size will always be 2
> >   
> 
> Basically readability... Yes, in this case it will be a stretch to assume we'll ever
> have anything bigger than 2 (so the scalability argument is not so applicable) so I'm
> ok if you don't pass the size. It's just I really dislike (as a practice) to have
> raw/magic numbers in the code. In here, it won't be that bad as by the context, one
> can easily understand the meaning of 2. Nevertheless, I would, still, at the very
> least consider to either use a #define or a better name for the iterator (anything
> more meaningful than 'i' so that it looks more understandable than 'i < 2')
> 

I'm late to the game, but I'd just split it into two parameters.
Code is shorter as well.

static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
					      unsigned int ain0, unsigned int ain1)
{
	if (ain0 >= st->info->num_inputs ||
	    ain1 >= st->info->num_inputs)
		return dev_err_probe(&st->sd.spi->dev, -EINVAL,
				     "Input pin number out of range for pair (%d %d).\n",
				     ain0, ain1);
	return 0;
}

> - Nuno Sá 
> >   
>