Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1300878

Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access

From Arnd Bergmann <arnd@arndb.de>
Newsgroups linux.kernel
Subject Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access
Date 2016-01-04 17:50 +0100
Message-ID <qNgut-5WH-1@gated-at.bofh.it> (permalink)
References <qMhiW-qy-7@gated-at.bofh.it> <qNf5n-59c-9@gated-at.bofh.it> <qNff4-5dA-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Monday 04 January 2016 15:20:58 Russell King - ARM Linux wrote:
> On Mon, Jan 04, 2016 at 04:17:47PM +0100, Arnd Bergmann wrote:
> > On Saturday 02 January 2016 14:17:46 Mark Brown wrote:
> > > On Sat, Jan 02, 2016 at 12:19:52AM +0100, Arnd Bergmann wrote:
> > > 
> > > > -             if (i2c_client->dev.of_node) {
> > > > +             if (IS_ENABLED(CONFIG_OF) && i2c_client->dev.of_node) {
> > > 
> > > This would be a lot nicer if there was an __always_null annotation we
> > > could put on of_node for !OF configurations, that'd Just Work and this
> > > can't be the only case where we have this idiom.
> > > 
> > 
> > How about an inline helper like
> > 
> > static inline struct device_node *dev_of_node(struct device *dev)
> > {
> >       if (IS_ENABLED(CONFIG_OF))
> >               return dev->of_node;    
> 
> ITYM:
> 
>         return IS_ENABLED(CONFIG_OF) ? dev->of_node : NULL;
> 
> or
> 
>         if (IS_ENABLED(CONFIG_OF))
>                 return dev->of_node;
>         else
>                 return NULL;
> 

Right, yes.

That reminds of a different problem that has been bugging me for a
while: We frequently have a pattern like

#ifdef CONFIG_FOO
static int function(void)
{
	...
}
#endif

struct operations = {
	...
#ifdef CONFIG_FOO
	.function = function;
#endif
	...
};

Except that people constantly get it wrong, e.g. by using the
wrong ifdef, forgetting one of the two ifdefs, or by leaving
unused static functions that only get called indirectly from the
other one that is built conditionally.

We could add a macro like

#define COND_PTR(config, ptr) (IS_ENABLED(config) ? (ptr) : NULL)

and then let the compiler figure out that "function" is unused even
without an explicit __maybe_unused annotation.  The function above
can be simplied to

static inline struct device_node *dev_of_node(struct device *dev)
{
	return COND_PTR(CONFIG_OF, dev->of_node);
}

with that, which is another benefit.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] ASoC: cs35l32: avoid uninitialized variable access Arnd Bergmann <arnd@arndb.de> - 2016-01-02 00:30 +0100
  Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access Mark Brown <broonie@kernel.org> - 2016-01-02 15:20 +0100
    Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access Arnd Bergmann <arnd@arndb.de> - 2016-01-04 16:20 +0100
      Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-01-04 16:30 +0100
        Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access Arnd Bergmann <arnd@arndb.de> - 2016-01-04 17:50 +0100
          Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-01-04 18:00 +0100
      Re: [PATCH] ASoC: cs35l32: avoid uninitialized variable access Mark Brown <broonie@kernel.org> - 2016-01-04 16:50 +0100

csiph-web