Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized

2016-11-13 Thread Greg KH
On Sun, Nov 13, 2016 at 09:47:41AM +0100, Greg KH wrote:
> On Sat, Nov 12, 2016 at 01:27:12PM +, Jonathan Cameron wrote:
> > On 11/11/16 19:49, Arnd Bergmann wrote:
> > > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> > >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann  wrote:
> > >>>
> > >>> Please merge these directly if you are happy with the result.
> > >>
> > >> I will take this.
> > > 
> > > Thanks a lot!
> > >  
> > >> I do see two warnings, but they both seem to be valid and recent,
> > >> though, so I have no issues with the spurious cases.
> > > 
> > > Ok, both of them should have my fixes coming your way already.
> > > 
> > >> Warning #1:
> > >>
> > >>   sound/soc/qcom/lpass-platform.c: In function 
> > >> ‘lpass_platform_pcmops_open’:
> > >>   sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> > >> uninitialized in this function [-Wmaybe-uninitialized]
> > >> drvdata->substream[dma_ch] = substream;
> > >> ~~~^~~
> > >>
> > >> and 'dma_ch' usage there really is crazy and wrong. Broken by
> > >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> > > 
> > > Right, the patches crossed here, the bugfix patch that introduced
> > > this came into linux-next over the kernel summit, and the fix I
> > > sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> > > before you pulled alsa tree. It should be fixed the next time you
> > > pull from the alsa tree, the commit is
> > > 
> > > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> > >  
> > >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> > >> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> > >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> > >> Align 16 bit big endian value of raw reads"), so you didn't see it.
> > > 
> > > This is the one I mentioned in the commit message as one that
> > > is fixed in linux-next and that should make it in soon.
> > > 
> > >>   drivers/iio/temperature/maxim_thermocouple.c: In function
> > >> ‘maxim_thermocouple_read_raw’:
> > >>   drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> > >> may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >> if (ret)
> > >>^
> > >>   drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> > >> declared here
> > >> int ret;
> > >> ^~~
> > >>
> > >> and I guess that code can just initialize 'ret' to '-EINVAL' or
> > >> something to just make the theoretical "somehow we had a wrong
> > >> chip->read_size" case error out cleanly.
> > > 
> > > Right, that was my conclusion too. I sent the bugfix on Oct 25
> > > for linux-next but it didn't make it in until this Monday, after
> > > you pulled the patch that introduced it on Oct 29.
> > > 
> > > The commit in staging-testing is
> > > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in 
> > > read()")
> > > 
> > > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> > > branch, so I suspect you were not planning to send this before the
> > > merge window. Could you make sure this ends up in v4.9 so we get
> > > a clean build when -Wmaybe-uninitialized gets enabled again?
> > I'll queue this up and send a pull to Greg tomorrow.
> > 
> > Was highly doubtful that a false warning suppression (be it an
> > understandable one) was worth sending mid cycle, hence it was
> > taking the slow route.
> 
> I can just cherry-pick this, no need to send a separate pull request.

Now done and sent to Linus, so all should be good here.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized

2016-11-13 Thread Greg KH
On Sat, Nov 12, 2016 at 01:27:12PM +, Jonathan Cameron wrote:
> On 11/11/16 19:49, Arnd Bergmann wrote:
> > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann  wrote:
> >>>
> >>> Please merge these directly if you are happy with the result.
> >>
> >> I will take this.
> > 
> > Thanks a lot!
> >  
> >> I do see two warnings, but they both seem to be valid and recent,
> >> though, so I have no issues with the spurious cases.
> > 
> > Ok, both of them should have my fixes coming your way already.
> > 
> >> Warning #1:
> >>
> >>   sound/soc/qcom/lpass-platform.c: In function 
> >> ‘lpass_platform_pcmops_open’:
> >>   sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> >> uninitialized in this function [-Wmaybe-uninitialized]
> >> drvdata->substream[dma_ch] = substream;
> >> ~~~^~~
> >>
> >> and 'dma_ch' usage there really is crazy and wrong. Broken by
> >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> > 
> > Right, the patches crossed here, the bugfix patch that introduced
> > this came into linux-next over the kernel summit, and the fix I
> > sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> > before you pulled alsa tree. It should be fixed the next time you
> > pull from the alsa tree, the commit is
> > 
> > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> >  
> >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> >> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> >> Align 16 bit big endian value of raw reads"), so you didn't see it.
> > 
> > This is the one I mentioned in the commit message as one that
> > is fixed in linux-next and that should make it in soon.
> > 
> >>   drivers/iio/temperature/maxim_thermocouple.c: In function
> >> ‘maxim_thermocouple_read_raw’:
> >>   drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> >> may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> if (ret)
> >>^
> >>   drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> >> declared here
> >> int ret;
> >> ^~~
> >>
> >> and I guess that code can just initialize 'ret' to '-EINVAL' or
> >> something to just make the theoretical "somehow we had a wrong
> >> chip->read_size" case error out cleanly.
> > 
> > Right, that was my conclusion too. I sent the bugfix on Oct 25
> > for linux-next but it didn't make it in until this Monday, after
> > you pulled the patch that introduced it on Oct 29.
> > 
> > The commit in staging-testing is
> > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in 
> > read()")
> > 
> > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> > branch, so I suspect you were not planning to send this before the
> > merge window. Could you make sure this ends up in v4.9 so we get
> > a clean build when -Wmaybe-uninitialized gets enabled again?
> I'll queue this up and send a pull to Greg tomorrow.
> 
> Was highly doubtful that a false warning suppression (be it an
> understandable one) was worth sending mid cycle, hence it was
> taking the slow route.

I can just cherry-pick this, no need to send a separate pull request.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized

2016-11-12 Thread Jonathan Cameron
On 11/11/16 19:49, Arnd Bergmann wrote:
> On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
>> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann  wrote:
>>>
>>> Please merge these directly if you are happy with the result.
>>
>> I will take this.
> 
> Thanks a lot!
>  
>> I do see two warnings, but they both seem to be valid and recent,
>> though, so I have no issues with the spurious cases.
> 
> Ok, both of them should have my fixes coming your way already.
> 
>> Warning #1:
>>
>>   sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
>>   sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>> drvdata->substream[dma_ch] = substream;
>> ~~~^~~
>>
>> and 'dma_ch' usage there really is crazy and wrong. Broken by
>> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> 
> Right, the patches crossed here, the bugfix patch that introduced
> this came into linux-next over the kernel summit, and the fix I
> sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> before you pulled alsa tree. It should be fixed the next time you
> pull from the alsa tree, the commit is
> 
> 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
>  
>> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
>> know that storage_bytes (chip->read_size) has to be 2/4. Again,
>> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
>> Align 16 bit big endian value of raw reads"), so you didn't see it.
> 
> This is the one I mentioned in the commit message as one that
> is fixed in linux-next and that should make it in soon.
> 
>>   drivers/iio/temperature/maxim_thermocouple.c: In function
>> ‘maxim_thermocouple_read_raw’:
>>   drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
>> may be used uninitialized in this function [-Wmaybe-uninitialized]
>> if (ret)
>>^
>>   drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
>> declared here
>> int ret;
>> ^~~
>>
>> and I guess that code can just initialize 'ret' to '-EINVAL' or
>> something to just make the theoretical "somehow we had a wrong
>> chip->read_size" case error out cleanly.
> 
> Right, that was my conclusion too. I sent the bugfix on Oct 25
> for linux-next but it didn't make it in until this Monday, after
> you pulled the patch that introduced it on Oct 29.
> 
> The commit in staging-testing is
> 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in 
> read()")
> 
> Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> branch, so I suspect you were not planning to send this before the
> merge window. Could you make sure this ends up in v4.9 so we get
> a clean build when -Wmaybe-uninitialized gets enabled again?
I'll queue this up and send a pull to Greg tomorrow.

Was highly doubtful that a false warning suppression (be it an
understandable one) was worth sending mid cycle, hence it was
taking the slow route.

Jonathan
> 
>   Arnd
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized

2016-11-11 Thread Arnd Bergmann
On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann  wrote:
> >
> > Please merge these directly if you are happy with the result.
> 
> I will take this.

Thanks a lot!
 
> I do see two warnings, but they both seem to be valid and recent,
> though, so I have no issues with the spurious cases.

Ok, both of them should have my fixes coming your way already.

> Warning #1:
> 
>   sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
>   sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drvdata->substream[dma_ch] = substream;
> ~~~^~~
> 
> and 'dma_ch' usage there really is crazy and wrong. Broken by
> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")

Right, the patches crossed here, the bugfix patch that introduced
this came into linux-next over the kernel summit, and the fix I
sent on Tuesday made it into Mark Brown's tree on Wednesday but not
before you pulled alsa tree. It should be fixed the next time you
pull from the alsa tree, the commit is

3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
 
> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> Align 16 bit big endian value of raw reads"), so you didn't see it.

This is the one I mentioned in the commit message as one that
is fixed in linux-next and that should make it in soon.

>   drivers/iio/temperature/maxim_thermocouple.c: In function
> ‘maxim_thermocouple_read_raw’:
>   drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> may be used uninitialized in this function [-Wmaybe-uninitialized]
> if (ret)
>^
>   drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> declared here
> int ret;
> ^~~
> 
> and I guess that code can just initialize 'ret' to '-EINVAL' or
> something to just make the theoretical "somehow we had a wrong
> chip->read_size" case error out cleanly.

Right, that was my conclusion too. I sent the bugfix on Oct 25
for linux-next but it didn't make it in until this Monday, after
you pulled the patch that introduced it on Oct 29.

The commit in staging-testing is
32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in read()")

Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
branch, so I suspect you were not planning to send this before the
merge window. Could you make sure this ends up in v4.9 so we get
a clean build when -Wmaybe-uninitialized gets enabled again?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized

2016-11-11 Thread Linus Torvalds
On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann  wrote:
>
> Please merge these directly if you are happy with the result.

I will take this.

I do see two warnings, but they both seem to be valid and recent,
though, so I have no issues with the spurious cases.

Warning #1:

  sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
  sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
drvdata->substream[dma_ch] = substream;
~~~^~~

and 'dma_ch' usage there really is crazy and wrong. Broken by
022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")

Warning #2 is not a real bug, but it's reasonable that gcc doesn't
know that storage_bytes (chip->read_size) has to be 2/4. Again,
introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
Align 16 bit big endian value of raw reads"), so you didn't see it.

  drivers/iio/temperature/maxim_thermocouple.c: In function
‘maxim_thermocouple_read_raw’:
  drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
may be used uninitialized in this function [-Wmaybe-uninitialized]
if (ret)
   ^
  drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
declared here
int ret;
^~~

and I guess that code can just initialize 'ret' to '-EINVAL' or
something to just make the theoretical "somehow we had a wrong
chip->read_size" case error out cleanly.

Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 00/11] getting back -Wmaybe-uninitialized

2016-11-10 Thread Arnd Bergmann
Hi Linus,

It took a while for some patches to make it into mainline through
maintainer trees, but the 28-patch series is now reduced to 10, with
one tiny patch added at the end.  I hope this can still make it into
v4.9. Aside from patches that are no longer required, I did these changes
compared to version 1:

- Dropped "iio: maxim_thermocouple: detect invalid storage size in
  read()", which is currently in linux-next as commit 32cb7d27e65d.
  This is the only remaining warning I see for a couple of corner
  cases (kbuild bot reports it on blackfin, kernelci bot and
  arm-soc bot both report it on arm64)

- Dropped "brcmfmac: avoid maybe-uninitialized warning in
  brcmf_cfg80211_start_ap", which is currently in net/master
  merge pending.

- Dropped two x86 patches, "x86: math-emu: possible uninitialized
  variable use" and "x86: mark target address as output in 'insb' asm"
  as they do not seem to trigger for a default build, and I got
  no feedback on them. Both of these are ancient issues and seem
  harmless, I will send them again to the x86 maintainers once
  the rest is merged.
  
- Dropped "rbd: false-postive gcc-4.9 -Wmaybe-uninitialized" based on
  feedback from Ilya Dryomov, who already has a different fix queued up
  for v4.10. The kbuild bot reports this as a warning for xtensa.
 
- Replaced "crypto: aesni: avoid -Wmaybe-uninitialized warning" with a
  simpler patch, this one always triggers but my first solution would not
  be safe for linux-4.9 any more at this point. I'll follow up with
  the larger patch as a cleanup for 4.10.
  
- Replaced "dib0700: fix nec repeat handling" with a better one,
  contributed by Sean Young.

Please merge these directly if you are happy with the result.

As the minimum, I'd hope to see the first patch get in soon,
but the individual bugfixes are hopefully now all appropriate
as well. If you see any regressions with the final patch, just
leave that one out and let me know what problems remain.

Arnd

Arnd Bergmann (10):
  Kbuild: enable -Wmaybe-uninitialized warning for "make W=1"
  NFSv4.1: work around -Wmaybe-uninitialized warning
  x86: apm: avoid uninitialized data
  nios2: fix timer initcall return value
  s390: pci: don't print uninitialized data for debugging
  [media] rc: print correct variable for z8f0811
  crypto: aesni: shut up -Wmaybe-uninitialized warning
  infiniband: shut up a maybe-uninitialized warning
  pcmcia: fix return value of soc_pcmcia_regulator_set
  Kbuild: enable -Wmaybe-uninitialized warnings by default

Sean Young (1):
  [media] dib0700: fix nec repeat handling

 Makefile | 10 +++---
 arch/arc/Makefile|  4 ++-
 arch/nios2/kernel/time.c |  1 +
 arch/s390/pci/pci_dma.c  |  2 +-
 arch/x86/crypto/aesni-intel_glue.c   |  4 +--
 arch/x86/kernel/apm_32.c |  5 ++-
 drivers/infiniband/core/cma.c| 54 +---
 drivers/media/i2c/ir-kbd-i2c.c   |  2 +-
 drivers/media/usb/dvb-usb/dib0700_core.c |  5 +--
 drivers/pcmcia/soc_common.c  |  2 +-
 fs/nfs/nfs4session.c | 10 +++---
 scripts/Makefile.extrawarn   |  1 +
 scripts/Makefile.ubsan   |  4 +++
 13 files changed, 61 insertions(+), 43 deletions(-)

-- 
2.9.0

Cc: Anna Schumaker 
Cc: "David S. Miller" 
Cc: Herbert Xu 
Cc: Ilya Dryomov 
Cc: Javier Martinez Canillas 
Cc: Jiri Kosina 
Cc: Jonathan Cameron 
Cc: Ley Foon Tan 
Cc: Luis R. Rodriguez 
Cc: Martin Schwidefsky 
Cc: Mauro Carvalho Chehab 
Cc: Michal Marek 
Cc: Russell King 
Cc: Sean Young 
Cc: Sebastian Ott 
Cc: Trond Myklebust 
Cc: x...@kernel.org
Cc: linux-kbu...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-snps-...@lists.infradead.org
Cc: nios2-...@lists.rocketboards.org
Cc: linux-s...@vger.kernel.org
Cc: linux-cry...@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linux-...@vger.kernel.org


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html