On Fri, 31 May 2024 09:10:43 +0200
Nuno Sá <noname.nuno@xxxxxxxxx> 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@xxxxxxxxxx>
> >
> > ...
> >
> > > > > +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á
> >
>
