Missing return check of of_property_read_*()

Missing return check of of_property_read_*()

[Takashi Iwai]

Hi,

I hoped that I wouldn't need to write this kind of post, but since
nothing happened so far, so here we go:

The following compile warnings are present for quite some time.  They
are due to the lack of return check from of_property_read*().

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;
               ^

sound/soc/sti/uniperif_player.c: In function ‘uni_player_init’:
sound/soc/sti/uniperif_player.c:1006:6: warning: ‘mode’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  if (strcasecmp(mode, "hdmi") == 0)
      ^
sound/soc/sti/uniperif_player.c:985:14: note: ‘mode’ was declared here
  const char *mode;
              ^

Both should be trivial to fix, but I won't bother to fix these things
by myself, as it results always in a matter of taste how to write.
So, please fix them appropriately.


Although not shown as compile warning like the above, lots of codec
drivers have no return value checks from of_property_read*(), too.
A quick grep reveals:

% git grep -B1 '^[[:space:]]*of_property_read_' sound/soc
sound/soc/codecs/adau1701.c-
sound/soc/codecs/adau1701.c:		of_property_read_u32(dev->of_node, "adi,pll-clkdiv",
--
sound/soc/codecs/adau1701.c-
sound/soc/codecs/adau1701.c:		of_property_read_u8_array(dev->of_node, "adi,pin-config",
--
sound/soc/codecs/cs35l32.c-
sound/soc/codecs/cs35l32.c:	of_property_read_u32(np, "cirrus,boost-manager", &val);
--
sound/soc/codecs/cs35l32.c-
sound/soc/codecs/cs35l32.c:	of_property_read_u32(np, "cirrus,sdout-datacfg", &val);
--
sound/soc/codecs/cs35l32.c-
sound/soc/codecs/cs35l32.c:	of_property_read_u32(np, "cirrus,battery-threshold", &val);
--
sound/soc/codecs/cs35l32.c-
sound/soc/codecs/cs35l32.c:	of_property_read_u32(np, "cirrus,battery-recovery", &val);
--
sound/soc/codecs/sta32x.c-
sound/soc/codecs/sta32x.c:	of_property_read_u8(np, "st,output-conf",
sound/soc/codecs/sta32x.c-			    &pdata->output_conf);
sound/soc/codecs/sta32x.c:	of_property_read_u8(np, "st,ch1-output-mapping",
sound/soc/codecs/sta32x.c-			    &pdata->ch1_output_mapping);
sound/soc/codecs/sta32x.c:	of_property_read_u8(np, "st,ch2-output-mapping",
sound/soc/codecs/sta32x.c-			    &pdata->ch2_output_mapping);
sound/soc/codecs/sta32x.c:	of_property_read_u8(np, "st,ch3-output-mapping",
--
sound/soc/codecs/sta32x.c-	tmp = 140;
sound/soc/codecs/sta32x.c:	of_property_read_u16(np, "st,drop-compensation-ns", &tmp);
--
sound/soc/codecs/sta350.c-
sound/soc/codecs/sta350.c:	of_property_read_u8(np, "st,output-conf",
sound/soc/codecs/sta350.c-			    &pdata->output_conf);
sound/soc/codecs/sta350.c:	of_property_read_u8(np, "st,ch1-output-mapping",
sound/soc/codecs/sta350.c-			    &pdata->ch1_output_mapping);
sound/soc/codecs/sta350.c:	of_property_read_u8(np, "st,ch2-output-mapping",
sound/soc/codecs/sta350.c-			    &pdata->ch2_output_mapping);
sound/soc/codecs/sta350.c:	of_property_read_u8(np, "st,ch3-output-mapping",
--
sound/soc/codecs/sta350.c-	tmp = 140;
sound/soc/codecs/sta350.c:	of_property_read_u16(np, "st,drop-compensation-ns", &tmp);
--
sound/soc/codecs/tas5086.c-
sound/soc/codecs/tas5086.c:		of_property_read_u32(of_node, "ti,charge-period",
--
sound/soc/codecs/tlv320aic31xx.c-
sound/soc/codecs/tlv320aic31xx.c:	of_property_read_u32(np, "ai31xx-micbias-vg", &value);
--
sound/soc/codecs/twl4030.c-
sound/soc/codecs/twl4030.c:	of_property_read_u32(node, "ti,digimic_delay",
sound/soc/codecs/twl4030.c-			     &pdata->digimic_delay);
sound/soc/codecs/twl4030.c:	of_property_read_u32(node, "ti,ramp_delay_value",
sound/soc/codecs/twl4030.c-			     &pdata->ramp_delay_value);
sound/soc/codecs/twl4030.c:	of_property_read_u32(node, "ti,offset_cncl_path",
--
sound/soc/fsl/fsl_esai.c-	esai_priv->synchronous =
sound/soc/fsl/fsl_esai.c:		of_property_read_bool(np, "fsl,esai-synchronous");
--
sound/soc/omap/omap-abe-twl6040.c-	priv->jack_detection = of_property_read_bool(node, "ti,jack-detection");
sound/soc/omap/omap-abe-twl6040.c:	of_property_read_u32(node, "ti,mclk-freq", &priv->mclk_freq);
--
sound/soc/samsung/i2s.c-	}
sound/soc/samsung/i2s.c:	of_property_read_string_index(dev->of_node,
--
sound/soc/sh/rcar/rsrc-card.c-	/* sampling rate convert */
sound/soc/sh/rcar/rsrc-card.c:	of_property_read_u32(node, "convert-rate", &priv->convert_rate);
--
sound/soc/sti/uniperif_player.c-
sound/soc/sti/uniperif_player.c:	of_property_read_u32(pnode, "version", &player->ver);
--
sound/soc/sti/uniperif_player.c-
sound/soc/sti/uniperif_player.c:	of_property_read_u32(pnode, "uniperiph-id", &info->id);
--
sound/soc/sti/uniperif_player.c-	/* Read the device mode property */
sound/soc/sti/uniperif_player.c:	of_property_read_string(pnode, "mode", &mode);
--
sound/soc/sti/uniperif_reader.c-
sound/soc/sti/uniperif_reader.c:	of_property_read_u32(node, "version", &reader->ver);


Many of them look really buggy (while some have the default value to
fall back).  And many codes have even no value checks.  These should
be fixed as well.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 849 bytes --]

On Wed, Sep 09, 2015 at 04:40:36PM +0200, Takashi Iwai wrote:

> I hoped that I wouldn't need to write this kind of post, but since
> nothing happened so far, so here we go:

Could you be more specific about nothing happening here...

> The following compile warnings are present for quite some time.  They
> are due to the lack of return check from of_property_read*().

> 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;
>                ^

How are you generating these - what compiler and config?  None of the
build bots are reporting any of the warnings you have here.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Missing return check of of_property_read_*()

[Brian Austin]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

On Wed, 9 Sep 2015, Mark Brown wrote:

> On Wed, Sep 09, 2015 at 04:40:36PM +0200, Takashi Iwai wrote:
> 
> > I hoped that I wouldn't need to write this kind of post, but since
> > nothing happened so far, so here we go:
> 
> Could you be more specific about nothing happening here...
> 
> > The following compile warnings are present for quite some time.  They
> > are due to the lack of return check from of_property_read*().
> 
> > 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;
> >                ^
> 
> How are you generating these - what compiler and config?  None of the
> build bots are reporting any of the warnings you have here.
> 
Shouldn't that build server at Intel catch these? I usually get emails 
about things like this from them.

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Missing return check of of_property_read_*()

[Takashi Iwai]

On Wed, 09 Sep 2015 16:55:53 +0200,
Brian Austin wrote:
> 
> On Wed, 9 Sep 2015, Mark Brown wrote:
> 
> > On Wed, Sep 09, 2015 at 04:40:36PM +0200, Takashi Iwai wrote:
> > 
> > > I hoped that I wouldn't need to write this kind of post, but since
> > > nothing happened so far, so here we go:
> > 
> > Could you be more specific about nothing happening here...
> > 
> > > The following compile warnings are present for quite some time.  They
> > > are due to the lack of return check from of_property_read*().
> > 
> > > 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;
> > >                ^
> > 
> > How are you generating these - what compiler and config?  None of the
> > build bots are reporting any of the warnings you have here.
> > 
> Shouldn't that build server at Intel catch these? I usually get emails 
> about things like this from them.

Maybe depending on gcc version.  But it's interesting if no other
static analyzer hits this, too.

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.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 725 bytes --]

On Wed, Sep 09, 2015 at 05:05:18PM +0200, Takashi Iwai wrote:
> Brian Austin wrote:

> > Shouldn't that build server at Intel catch these? I usually get emails 
> > about things like this from them.

> Maybe depending on gcc version.  But it's interesting if no other
> static analyzer hits this, too.

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.

> 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...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Missing return check of of_property_read_*()

[Takashi Iwai]

On Wed, 09 Sep 2015 17:57:37 +0200,
Mark Brown wrote:
> 
> On Wed, Sep 09, 2015 at 05:05:18PM +0200, Takashi Iwai wrote:
> > Brian Austin wrote:
> 
> > > Shouldn't that build server at Intel catch these? I usually get emails 
> > > about things like this from them.
> 
> > Maybe depending on gcc version.  But it's interesting if no other
> > static analyzer hits this, too.
> 
> 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.

> > 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.


Takashi

Re: Missing return check of of_property_read_*()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1179 bytes --]

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.  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.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: 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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Fengguang Wu]

Hi Takashi,

> 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?

It's covered through the lots of randconfig tests. However the problem
is, they are pretty old warnings and 0day ignores old warnings because
old warnings may well be intensionally ignored by people. On the
contrast, build errors can be re-reported if remain unfixed for long time.

Thanks,
Fengguang
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Fengguang Wu]

On Thu, Sep 10, 2015 at 11:10:22AM +0800, Fengguang Wu wrote:
> Hi Takashi,
> 
> > 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?
> 
> It's covered through the lots of randconfig tests. However the problem
> is, they are pretty old warnings and 0day ignores old warnings because
> old warnings may well be intensionally ignored by people. On the
> contrast, build errors can be re-reported if remain unfixed for long time.

That said, if as a maintainer you demand "all warnings should be quieted",
I'll happily help you reminding the people who break the rule.

If it's a generally agreed rule by the maintainers, I'll be happy to
help guarantee it kernel wide.

Thanks,
Fengguang
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Takashi Iwai]

On Thu, 10 Sep 2015 05:23:44 +0200,
Fengguang Wu wrote:
> 
> On Thu, Sep 10, 2015 at 11:10:22AM +0800, Fengguang Wu wrote:
> > Hi Takashi,
> > 
> > > 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?
> > 
> > It's covered through the lots of randconfig tests. However the problem
> > is, they are pretty old warnings and 0day ignores old warnings because
> > old warnings may well be intensionally ignored by people. On the
> > contrast, build errors can be re-reported if remain unfixed for long time.
> 
> That said, if as a maintainer you demand "all warnings should be quieted",
> I'll happily help you reminding the people who break the rule.
> 
> If it's a generally agreed rule by the maintainers, I'll be happy to
> help guarantee it kernel wide.

Well, there can be no black-or-white answer for this, as you know.
Indeed, what annoys most is repeatedly mail combs for minor or false
positive warnings.  But sometimes checking warnings is useful; when I
look through build warnings occasionally, it really hits real bugs
sometimes.

So this made me wonder whether we have a way to take a glance through
gathered compile warnings at once.  Are they archived and reachable?


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Takashi Iwai]

On Thu, 10 Sep 2015 07:32:04 +0200,
Takashi Iwai wrote:
> 
> On Thu, 10 Sep 2015 05:23:44 +0200,
> Fengguang Wu wrote:
> > 
> > On Thu, Sep 10, 2015 at 11:10:22AM +0800, Fengguang Wu wrote:
> > > Hi Takashi,
> > > 
> > > > 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?
> > > 
> > > It's covered through the lots of randconfig tests. However the problem
> > > is, they are pretty old warnings and 0day ignores old warnings because
> > > old warnings may well be intensionally ignored by people. On the
> > > contrast, build errors can be re-reported if remain unfixed for long time.
> > 
> > That said, if as a maintainer you demand "all warnings should be quieted",
> > I'll happily help you reminding the people who break the rule.
> > 
> > If it's a generally agreed rule by the maintainers, I'll be happy to
> > help guarantee it kernel wide.
> 
> Well, there can be no black-or-white answer for this, as you know.
> Indeed, what annoys most is repeatedly mail combs for minor or false

Oops, s/comb/bomb/
It's quite opposite...


Takashi

> positive warnings.  But sometimes checking warnings is useful; when I
> look through build warnings occasionally, it really hits real bugs
> sometimes.
> 
> So this made me wonder whether we have a way to take a glance through
> gathered compile warnings at once.  Are they archived and reachable?
> 
> 
> thanks,
> 
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Fengguang Wu]

On Thu, Sep 10, 2015 at 07:32:04AM +0200, Takashi Iwai wrote:
> On Thu, 10 Sep 2015 05:23:44 +0200,
> Fengguang Wu wrote:
> > 
> > On Thu, Sep 10, 2015 at 11:10:22AM +0800, Fengguang Wu wrote:
> > > Hi Takashi,
> > > 
> > > > 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?
> > > 
> > > It's covered through the lots of randconfig tests. However the problem
> > > is, they are pretty old warnings and 0day ignores old warnings because
> > > old warnings may well be intensionally ignored by people. On the
> > > contrast, build errors can be re-reported if remain unfixed for long time.
> > 
> > That said, if as a maintainer you demand "all warnings should be quieted",
> > I'll happily help you reminding the people who break the rule.
> > 
> > If it's a generally agreed rule by the maintainers, I'll be happy to
> > help guarantee it kernel wide.
> 
> Well, there can be no black-or-white answer for this, as you know.
> Indeed, what annoys most is repeatedly mail combs for minor or false
> positive warnings.  But sometimes checking warnings is useful; when I
> look through build warnings occasionally, it really hits real bugs
> sometimes.

That's true.

> So this made me wonder whether we have a way to take a glance through
> gathered compile warnings at once.  Are they archived and reachable?

Yes. Here are the active "may be used uninitialized" warnings under sound/:

sound/core/pcm_lib.c:244: warning: 'curr_tstamp.tv_nsec' may be used uninitialized in this function
sound/core/pcm_lib.c:244: warning: 'curr_tstamp.tv_sec' may be used uninitialized in this function
sound/drivers/serial-u16550.c:966:9: warning: 'uart' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/firewire/bebob/bebob_focusrite.c:175:14: warning: 'value' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/asihpi/hpi6205.c:605:5: warning: 'p_control_cache_virtual' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/asihpi/hpioctl.c:304:17: warning: 'puhr' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/atiixp.c:1683:14: warning: 'chip' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/atiixp_modem.c:1314:2: warning: 'chip' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/hda/hda_generic.c:2872:32: warning: 'mix_val' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/hda/hda_generic.c:2875:305: warning: 'mute_val' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/hda/patch_realtek.c:4826:12: warning: 'pin' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/hda/patch_via.c:2108:24: warning: 'dac' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/lx6464es/lx_core.c:1121:31: warning: 'irqsrc' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/pcxhr/pcxhr.c:259:25: warning: 'pllreg' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/pcxhr/pcxhr.c:337:6: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/pcxhr/pcxhr.c:342:24: warning: 'realfreq' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/pci/via82xx.c:2589:2: warning: 'chip' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/soc/codecs/arizona.c:1367:27: warning: 'aif_tx_state' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/soc/codecs/arizona.c:1370:21: warning: 'aif_rx_state' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/soc/codecs/cs35l32.c:329:2: warning: 'val' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/soc/codecs/wm2000.c:543:6: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/soc/codecs/wm8978.c:814:5: warning: 'diff' may be used uninitialized in this function [-Wuninitialized]
sound/soc/fsl/fsl_ssi.c:715:7: warning: 'baudrate' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/soc/generic/simple-card.c:476:29: warning: 'flags' may be used uninitialized in this function [-Wuninitialized]
sound/soc/soc-core.c:1768:7: warning: 'compress_type' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/soc/soc-dapm.c:3125:6: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/usb/caiaq/device.c:494:16: warning: 'card' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/usb/caiaq/device.h:128:51: warning: 'card' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/usb/format.c:482:5: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/usb/usbaudio.c:2812:5: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/usb/usx2y/us122l.c:583:15: warning: 'card' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/usb/usx2y/us122l.h:20:40: warning: 'card' may be used uninitialized in this function [-Wmaybe-uninitialized]
sound/usb/usx2y/usbusx2y.c:390:15: warning: 'card' may be used uninitialized in this function [-Wmaybe-uninitialized]

Thanks,
Fengguang
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 514 bytes --]

On Thu, Sep 10, 2015 at 02:24:32PM +0800, Fengguang Wu wrote:
> On Thu, Sep 10, 2015 at 07:32:04AM +0200, Takashi Iwai wrote:

> > So this made me wonder whether we have a way to take a glance through
> > gathered compile warnings at once.  Are they archived and reachable?

> Yes. Here are the active "may be used uninitialized" warnings under sound/:

> sound/core/pcm_lib.c:244: warning: 'curr_tstamp.tv_nsec' may be used uninitialized in this function

Is there any way we can view the current status on line?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Missing return check of of_property_read_*()

[Fengguang Wu]

On Thu, Sep 10, 2015 at 11:54:50AM +0100, Mark Brown wrote:
> On Thu, Sep 10, 2015 at 02:24:32PM +0800, Fengguang Wu wrote:
> > On Thu, Sep 10, 2015 at 07:32:04AM +0200, Takashi Iwai wrote:
> 
> > > So this made me wonder whether we have a way to take a glance through
> > > gathered compile warnings at once.  Are they archived and reachable?
> 
> > Yes. Here are the active "may be used uninitialized" warnings under sound/:
> 
> > sound/core/pcm_lib.c:244: warning: 'curr_tstamp.tv_nsec' may be used uninitialized in this function
> 
> Is there any way we can view the current status on line?

We don't have interactive web interfaces to query/browse things.
Sorry!

Regards,
Fengguang

Re: Missing return check of of_property_read_*()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 496 bytes --]

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.

We should start getting some coverage for this from kernelci.org soon
(and x86 allmodconfig with OF disabled).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Missing return check of of_property_read_*()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 645 bytes --]

On Wed, Sep 09, 2015 at 09:55:53AM -0500, Brian Austin wrote:
> On Wed, 9 Sep 2015, Mark Brown wrote:
> > On Wed, Sep 09, 2015 at 04:40:36PM +0200, Takashi Iwai wrote:

> > How are you generating these - what compiler and config?  None of the
> > build bots are reporting any of the warnings you have here.

> Shouldn't that build server at Intel catch these? I usually get emails 
> about things like this from them.

Yeah, I haven't particularly noticed anything from them either (they
don't publish reporting like kernelci.org does or I have locally for
mine so it's harder to see if they reported something and it got
missed).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Missing return check of of_property_read_*()

[Takashi Iwai]

On Wed, 09 Sep 2015 16:51:58 +0200,
Mark Brown wrote:
> 
> On Wed, Sep 09, 2015 at 04:40:36PM +0200, Takashi Iwai wrote:
> 
> > I hoped that I wouldn't need to write this kind of post, but since
> > nothing happened so far, so here we go:
> 
> Could you be more specific about nothing happening here...

A wonderful gift such like a devoted maintainer/developer already
fixing these bugs magically :)

> > The following compile warnings are present for quite some time.  They
> > are due to the lack of return check from of_property_read*().
> 
> > 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;
> >                ^
> 
> How are you generating these - what compiler and config?  None of the
> build bots are reporting any of the warnings you have here.

Then maybe this can be seen since gcc 5.x.  A good progress of gcc.
I've seen this since the beginning of merge for cs35l32.c (and I
reported this already at merging the branch).

There is nothing special in config or compiler option, just use the
normal x86-64 build with CONFIG_COMPILE_TEST.  I'm using gcc-5.1.1 for
now.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Lars-Peter Clausen]

On 09/09/2015 05:01 PM, Takashi Iwai wrote:
> On Wed, 09 Sep 2015 16:51:58 +0200,
> Mark Brown wrote:
>>
>> On Wed, Sep 09, 2015 at 04:40:36PM +0200, Takashi Iwai wrote:
>>
>>> I hoped that I wouldn't need to write this kind of post, but since
>>> nothing happened so far, so here we go:
>>
>> Could you be more specific about nothing happening here...
> 
> A wonderful gift such like a devoted maintainer/developer already
> fixing these bugs magically :)
> 
>>> The following compile warnings are present for quite some time.  They
>>> are due to the lack of return check from of_property_read*().
>>
>>> 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;
>>>                ^
>>
>> How are you generating these - what compiler and config?  None of the
>> build bots are reporting any of the warnings you have here.
> 
> Then maybe this can be seen since gcc 5.x.  A good progress of gcc.
> I've seen this since the beginning of merge for cs35l32.c (and I
> reported this already at merging the branch).
> 
> There is nothing special in config or compiler option, just use the
> normal x86-64 build with CONFIG_COMPILE_TEST.  I'm using gcc-5.1.1 for
> now.

I think you'll get them for any config with CONFIG_OF disabled. I've been
seeing those warnings for a while now when compile-testing for x86 and I'm
still on gcc 4.9.

But even if you don't get the warning the code is still broken, the compiler
just does not know.

- Lars

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Takashi Iwai]

On Wed, 09 Sep 2015 17:05:05 +0200,
Lars-Peter Clausen wrote:
> 
> On 09/09/2015 05:01 PM, Takashi Iwai wrote:
> > On Wed, 09 Sep 2015 16:51:58 +0200,
> > Mark Brown wrote:
> >>
> >> On Wed, Sep 09, 2015 at 04:40:36PM +0200, Takashi Iwai wrote:
> >>
> >>> I hoped that I wouldn't need to write this kind of post, but since
> >>> nothing happened so far, so here we go:
> >>
> >> Could you be more specific about nothing happening here...
> > 
> > A wonderful gift such like a devoted maintainer/developer already
> > fixing these bugs magically :)
> > 
> >>> The following compile warnings are present for quite some time.  They
> >>> are due to the lack of return check from of_property_read*().
> >>
> >>> 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;
> >>>                ^
> >>
> >> How are you generating these - what compiler and config?  None of the
> >> build bots are reporting any of the warnings you have here.
> > 
> > Then maybe this can be seen since gcc 5.x.  A good progress of gcc.
> > I've seen this since the beginning of merge for cs35l32.c (and I
> > reported this already at merging the branch).
> > 
> > There is nothing special in config or compiler option, just use the
> > normal x86-64 build with CONFIG_COMPILE_TEST.  I'm using gcc-5.1.1 for
> > now.
> 
> I think you'll get them for any config with CONFIG_OF disabled. I've been
> seeing those warnings for a while now when compile-testing for x86 and I'm
> still on gcc 4.9.

Ah, good point.  That can trigger the check easily, of course.
But then I don't know why 0day check didn't catch these.

> But even if you don't get the warning the code is still broken, the compiler
> just does not know.

Right.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Missing return check of of_property_read_*()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 944 bytes --]

On Wed, Sep 09, 2015 at 05:05:05PM +0200, Lars-Peter Clausen wrote:
> On 09/09/2015 05:01 PM, Takashi Iwai wrote:

> > There is nothing special in config or compiler option, just use the
> > normal x86-64 build with CONFIG_COMPILE_TEST.  I'm using gcc-5.1.1 for
> > now.

> I think you'll get them for any config with CONFIG_OF disabled. I've been
> seeing those warnings for a while now when compile-testing for x86 and I'm
> still on gcc 4.9.

Oh, right - that'll be the issue, nobody build tests those
configurations as standard, all the builders are all*config or
configurations that don't include ASoC.  I'd assume the Intel people are
seeing these in their testing but otherwise everyone is going to be
using DT and not seeing warnings.

> But even if you don't get the warning the code is still broken, the compiler
> just does not know.

Sure, I'm not questioning that there's an issue but rather why nobody
else had seen the warnings.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Question about patch ASoC: Prevent components from being bound to multiple cards

[Koro Chen]

Hi Lars 

I have two cards originally but the second one failed because of the
patch ASoC: Prevent components from being bound to multiple cards.

My configuration is like this:
card0: use platform's DAI component A
card1: use the same platform's DAI component B

The two cards use different DAI components, so it should be OK but still
gets failed. I looked into this and found it is because
snd_soc_register_platform() itself will also add a component, and this
component was bound to the first card so the second probe failed.

It seems like I might need to separate the platform into two platform
drivers, but it is strange since I have only one audio hardware. I
wonder if it is reasonable to prevent a single ASoC platform driver from
being used by multiple cards? Do you have any suggestion how to fix
this?

Re: Question about patch ASoC: Prevent components from being bound to multiple cards

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 404 bytes --]

On Fri, Oct 02, 2015 at 06:34:24PM +0800, Koro Chen wrote:

> I have two cards originally but the second one failed because of the
> patch ASoC: Prevent components from being bound to multiple cards.

> My configuration is like this:
> card0: use platform's DAI component A
> card1: use the same platform's DAI component B

What do you mean by "DAI component" here?  It's not a term I'm familiar
with...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Question about patch ASoC: Prevent components from being bound to multiple cards

[Koro Chen]

On Fri, 2015-10-02 at 15:48 +0100, Mark Brown wrote:
> On Fri, Oct 02, 2015 at 06:34:24PM +0800, Koro Chen wrote:
> 
> > I have two cards originally but the second one failed because of the
> > patch ASoC: Prevent components from being bound to multiple cards.
> 
> > My configuration is like this:
> > card0: use platform's DAI component A
> > card1: use the same platform's DAI component B
> 
> What do you mean by "DAI component" here?  It's not a term I'm familiar
> with...
Sorry I didn't make myself clear enough. In the
sound/soc/mediatek/mtk-afe-pcm.c, I register two components
"mtk_afe_pcm_dai_component" and "mtk_afe_hdmi_dai_component", each has
its corresponding DAIs. Card0 (which is mt8173-rt5676-rt5650.c) uses
DAIs from the first component, and card1 (new) uses DAIs of the second
component. These two components are probed successfully and separately
by card0 and card1, but card1 failed to probe another component created
by snd_soc_register_platform(), which is already bound to card0.
 

Re: Question about patch ASoC: Prevent components from being bound to multiple cards

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1365 bytes --]

On Sat, Oct 03, 2015 at 11:01:40PM +0800, Koro Chen wrote:
> On Fri, 2015-10-02 at 15:48 +0100, Mark Brown wrote:
> > On Fri, Oct 02, 2015 at 06:34:24PM +0800, Koro Chen wrote:

> > > I have two cards originally but the second one failed because of the
> > > patch ASoC: Prevent components from being bound to multiple cards.

> > > My configuration is like this:
> > > card0: use platform's DAI component A
> > > card1: use the same platform's DAI component B

> > What do you mean by "DAI component" here?  It's not a term I'm familiar
> > with...

> Sorry I didn't make myself clear enough. In the
> sound/soc/mediatek/mtk-afe-pcm.c, I register two components
> "mtk_afe_pcm_dai_component" and "mtk_afe_hdmi_dai_component", each has
> its corresponding DAIs. Card0 (which is mt8173-rt5676-rt5650.c) uses
> DAIs from the first component, and card1 (new) uses DAIs of the second
> component. These two components are probed successfully and separately
> by card0 and card1, but card1 failed to probe another component created
> by snd_soc_register_platform(), which is already bound to card0.

OK, I see.  We probably do want to relax that restriction, it makes more
sense at the DAI level but at the component level when we have
components that have a bunch of independant DAIs hanging off a DMA
controler as one component (which does make sense) then it breaks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Question about patch ASoC: Prevent components from being bound to multiple cards

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1163 bytes --]

On Mon, Oct 05, 2015 at 11:07:17AM +0100, Mark Brown wrote:
> On Sat, Oct 03, 2015 at 11:01:40PM +0800, Koro Chen wrote:

> > Sorry I didn't make myself clear enough. In the
> > sound/soc/mediatek/mtk-afe-pcm.c, I register two components
> > "mtk_afe_pcm_dai_component" and "mtk_afe_hdmi_dai_component", each has
> > its corresponding DAIs. Card0 (which is mt8173-rt5676-rt5650.c) uses
> > DAIs from the first component, and card1 (new) uses DAIs of the second
> > component. These two components are probed successfully and separately
> > by card0 and card1, but card1 failed to probe another component created
> > by snd_soc_register_platform(), which is already bound to card0.

> OK, I see.  We probably do want to relax that restriction, it makes more
> sense at the DAI level but at the component level when we have
> components that have a bunch of independant DAIs hanging off a DMA
> controler as one component (which does make sense) then it breaks.

Oh, one other thing here - if there is routing within your component
which would allow the two cards to be interconnected then that's a bit
different and the cards should probably be combined into one.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Question about patch ASoC: Prevent components from being bound to multiple cards

[Koro Chen]

On Mon, 2015-10-05 at 11:07 +0100, Mark Brown wrote:
> On Sat, Oct 03, 2015 at 11:01:40PM +0800, Koro Chen wrote:
> > On Fri, 2015-10-02 at 15:48 +0100, Mark Brown wrote:
> > > On Fri, Oct 02, 2015 at 06:34:24PM +0800, Koro Chen wrote:
> 
> > > > I have two cards originally but the second one failed because of the
> > > > patch ASoC: Prevent components from being bound to multiple cards.
> 
> > > > My configuration is like this:
> > > > card0: use platform's DAI component A
> > > > card1: use the same platform's DAI component B
> 
> > > What do you mean by "DAI component" here?  It's not a term I'm familiar
> > > with...
> 
> > Sorry I didn't make myself clear enough. In the
> > sound/soc/mediatek/mtk-afe-pcm.c, I register two components
> > "mtk_afe_pcm_dai_component" and "mtk_afe_hdmi_dai_component", each has
> > its corresponding DAIs. Card0 (which is mt8173-rt5676-rt5650.c) uses
> > DAIs from the first component, and card1 (new) uses DAIs of the second
> > component. These two components are probed successfully and separately
> > by card0 and card1, but card1 failed to probe another component created
> > by snd_soc_register_platform(), which is already bound to card0.
> 
> OK, I see.  We probably do want to relax that restriction, it makes more
> sense at the DAI level but at the component level when we have
> components that have a bunch of independant DAIs hanging off a DMA
> controler as one component (which does make sense) then it breaks.
Thanks for your suggestion. Is it make sense if we use
component->registered_as_component, which is only true in case of
snd_soc_register_component() and will be false for components created by
snd_soc_register_platform() or snd_soc_register_codec(), then modify the
check condition like this:

@@ -1102,11 +1102,12 @@ static int soc_probe_component(struct
snd_soc_card *card,
        struct snd_soc_dai *dai;
        int ret;

        if (!strcmp(component->name, "snd-soc-dummy"))
                return 0;

        if (component->card) {
-               if (component->card != card) {
+               if (component->card != card &&
component->registered_as_component) {