Re: [PATCH v4 02/13] iio: adc: ad4134: Sign extend sample data

[Marcelo Schmitt <marcelo.schmitt1@gmail.com>]

On 10/06, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] build: -Wmaybe-uninitialized warning for conditionally assigned 'sample'
> --
> 
> commit 6ed41c1a7f4a36c8c7537ebc2c932623b78e2afa
> Author: Marcelo Schmitt <marcelo.schmitt@analog.com>
> 
> iio: adc: ad4134: Sign extend sample data
> 
> Sign extend ADC sample data for correct delivery of negative ADC input
> difference.
> 
> > diff --git a/drivers/iio/adc/ad4134.c b/drivers/iio/adc/ad4134.c
> > index e53c10b9a44a9..6fdeb27d7a42b 100644
> > --- a/drivers/iio/adc/ad4134.c
> > +++ b/drivers/iio/adc/ad4134.c
> > @@ -172,6 +172,7 @@ static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,
> >  			    unsigned int *val)
> >  {
> >  	unsigned int i;
> > +	u32 sample;
> >  	int ret;
> >  
> >  	/*
> > @@ -196,8 +197,9 @@ static int ad4134_data_read(struct ad4134_state *st, unsigned int reg,
> >  		 * Clock out data from all channels to avoid that.
> >  		 */
> >  		if (i == AD4134_VREG_CH(reg))
> > -			*val = get_unaligned_be24(st->rx_buf);
> > +			sample = get_unaligned_be24(st->rx_buf);
> >  	}
> > +	*val = sign_extend32(sample, AD4134_CHAN_PRECISION_BITS - 1);
>                              ^^^^^^
> 
> [Severity: Low]
> Does this code trigger a -Wmaybe-uninitialized compiler warning?
> 
> The local variable sample is declared without initialization and is only
> assigned conditionally if i == AD4134_VREG_CH(reg). While the regmap core
> strictly validates reg before invoking the read callback, the compiler analyzes
> ad4134_data_read() locally and cannot statically deduce this external bounds
> guarantee.
> 
> Consequently, the compiler evaluates a theoretical path where the condition is
> never met, leaving sample unassigned before it is passed to sign_extend32().
> 
> This can cause build failures in kernel configurations with CONFIG_WERROR
> enabled. Explicitly initializing u32 sample = 0; or unconditionally performing
> the sign extension inside the if block would resolve this.

Okay, u32 sample = 0 is what I've chosen for v5.
> 
> >  
> >  	return 0;
> >  }