| Message ID | 20190307171604.27791-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: adc: stmpe-adc: Shuffle an if statement around in stmpe_adc_isr | expand |
On Thu, Mar 7, 2019 at 9:16 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > When building with -Wsometimes-uninitialized, Clang warns: > > drivers/iio/adc/stmpe-adc.c:204:13: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] > > Clang can't tell that data will never be used uninitialized because the > two if statements take care of all cases. Remove the first if statement > and make it the else branch of the second one so that it is apparent to > Clang that all cases are covered. > > Link: https://github.com/ClangBuiltLinux/linux/issues/387 > Suggested-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> LGTM, thanks Nathan. Reviewed-by: NIck Desaulniers <ndesaulniers@google.com> > --- > drivers/iio/adc/stmpe-adc.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c > index 37f4b74a5d32..7921f827c6ec 100644 > --- a/drivers/iio/adc/stmpe-adc.c > +++ b/drivers/iio/adc/stmpe-adc.c > @@ -184,9 +184,6 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id) > struct stmpe_adc *info = (struct stmpe_adc *)dev_id; > u16 data; > > - if (info->channel > STMPE_TEMP_CHANNEL) > - return IRQ_NONE; > - > if (info->channel <= STMPE_ADC_LAST_NR) { > int int_sta; > > @@ -205,6 +202,8 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id) > /* Read value */ > stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2, > (u8 *) &data); > + } else { > + return IRQ_NONE; > } > > info->value = (u32) be16_to_cpu(data); > -- > 2.21.0 >
On Thu, 7 Mar 2019 10:31:55 -0800 Nick Desaulniers <ndesaulniers@google.com> wrote: > On Thu, Mar 7, 2019 at 9:16 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: > > > > When building with -Wsometimes-uninitialized, Clang warns: > > > > drivers/iio/adc/stmpe-adc.c:204:13: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] > > > > Clang can't tell that data will never be used uninitialized because the > > two if statements take care of all cases. Remove the first if statement > > and make it the else branch of the second one so that it is apparent to > > Clang that all cases are covered. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/387 > > Suggested-by: Nick Desaulniers <ndesaulniers@google.com> > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > LGTM, thanks Nathan. > Reviewed-by: NIck Desaulniers <ndesaulniers@google.com> Agreed. Seems obviously correct. Stefan, I'm only pushing this out as testing for now so happy to rebase if you have comments. Thanks, Jonathan > > > --- > > drivers/iio/adc/stmpe-adc.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c > > index 37f4b74a5d32..7921f827c6ec 100644 > > --- a/drivers/iio/adc/stmpe-adc.c > > +++ b/drivers/iio/adc/stmpe-adc.c > > @@ -184,9 +184,6 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id) > > struct stmpe_adc *info = (struct stmpe_adc *)dev_id; > > u16 data; > > > > - if (info->channel > STMPE_TEMP_CHANNEL) > > - return IRQ_NONE; > > - > > if (info->channel <= STMPE_ADC_LAST_NR) { > > int int_sta; > > > > @@ -205,6 +202,8 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id) > > /* Read value */ > > stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2, > > (u8 *) &data); > > + } else { > > + return IRQ_NONE; > > } > > > > info->value = (u32) be16_to_cpu(data); > > -- > > 2.21.0 > > > >
diff --git a/drivers/iio/adc/stmpe-adc.c b/drivers/iio/adc/stmpe-adc.c index 37f4b74a5d32..7921f827c6ec 100644 --- a/drivers/iio/adc/stmpe-adc.c +++ b/drivers/iio/adc/stmpe-adc.c @@ -184,9 +184,6 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id) struct stmpe_adc *info = (struct stmpe_adc *)dev_id; u16 data; - if (info->channel > STMPE_TEMP_CHANNEL) - return IRQ_NONE; - if (info->channel <= STMPE_ADC_LAST_NR) { int int_sta; @@ -205,6 +202,8 @@ static irqreturn_t stmpe_adc_isr(int irq, void *dev_id) /* Read value */ stmpe_block_read(info->stmpe, STMPE_REG_TEMP_DATA, 2, (u8 *) &data); + } else { + return IRQ_NONE; } info->value = (u32) be16_to_cpu(data);
When building with -Wsometimes-uninitialized, Clang warns: drivers/iio/adc/stmpe-adc.c:204:13: warning: variable 'data' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] Clang can't tell that data will never be used uninitialized because the two if statements take care of all cases. Remove the first if statement and make it the else branch of the second one so that it is apparent to Clang that all cases are covered. Link: https://github.com/ClangBuiltLinux/linux/issues/387 Suggested-by: Nick Desaulniers <ndesaulniers@google.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/iio/adc/stmpe-adc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)