[alsa-devel] Missing return check of of_property_read_*()

[Takashi Iwai]

On Wed, 09 Sep 2015 18:19:19 +0200,
Mark Brown wrote:
> 
> On Wed, Sep 09, 2015 at 06:01:22PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > It'll be because nobody is checking !OF ASoC configurations and paying
> > > attention to warnings - with !OF the functions are inline in the header
> > > and the compiler can tell nothing happens.
> 
> > Yes, but non-OF ASoC gets more popular on x86 nowadays, so this should
> > be covered regularly, too.
> 
> Hopefully Intel will be paying attention here!  :P
> 
> > > > I'm considering whether adding __must_check to of_property_read*()
> > > > would make sense.  Then it'll be caught no matter which gcc version is
> > > > used.  But this would hit too many spots.
> 
> > > Well, if they're all buggy...
> 
> > Not all of them are buggy, of course.  Some codes set the default
> > fallback value beforehand.  But, if thinking of future use, it might
> > be better to make the check mandatory.  No strong opinion here,
> > though.
> 
> I don't know how clear it is that a failed read won't overwrite existing
> data in the value variable.

Hm, right, this doesn't seem clearly defined although implicitly
assumed by some callers.  Maybe the explicit error check is safer and
more understandable.

> The other option would be to have Kconfig
> able to generate all*config with select options (like OF) disabled and
> then get the build bots to start covering those as a standard
> configuration.

Agreed.

Fengguang, we've been discussing about the compile warnings that
weren't caught by 0days.  It seems that it's triggered by !CONFIG_OF
but with CONFIG_COMPILE_TEST=y.  Then I got warnings like:

sound/soc/codecs/cs35l32.c: In function ‘cs35l32_i2c_probe’:
sound/soc/codecs/cs35l32.c:278:2: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  switch (val) {
  ^
sound/soc/codecs/cs35l32.c:272:15: note: ‘val’ was declared here
  unsigned int val;
               ^

The above was with gcc-5.1.1, but Lars told that he saw such a warning
with gcc-4.9, too.

Could you add this kind of kconfig in your test?


thanks,

Takashi