Re: [Linux-eng] [RFC 0/3] Abstract empty functions with STUB_UNLESS macro
From: Andrew Murray
Date: Fri Jan 18 2019 - 12:01:23 EST
On Fri, Jan 18, 2019 at 04:44:25PM +0000, Dave Martin wrote:
> On Fri, Jan 18, 2019 at 04:37:36PM +0000, Russell King - ARM Linux admin wrote:
> > On Fri, Jan 18, 2019 at 04:00:27PM +0000, Andrew Murray wrote:
> > > A common pattern found in header files is a function declaration dependent
> > > on a CONFIG_ option being enabled, followed by an empty function for when
> > > that option isn't enabled. This boilerplate code can often take up a lot
> > > of space and impact code readability.
> > >
> > > This series introduces a STUB_UNLESS macro that simplifies header files as
> > > follows:
> > >
> > > STUB_UNLESS(CONFIG_FOO, [body], prototype)
> >
> > Can you explain the desire to make the second argument optional,
> > rather than having the mandatory arguments first and the optional body
> > last? It will mean more lines at each site, but I don't think that's
> > a bad thing:
My intent was to make the function prototype look like an ordinary prototype
but with all this special macro stuff on the preceding line. Much like this:
> >
> > STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
> > void hw_breakpoint_thread_switch(struct task_struct *next));
Besides the extra ')' at the end it looks like a normal prototype. I felt this
may be important as existing tooling (ctags etc) might have a better chance of
recognising it and it wouldn't be so alien to new developers. I feared that if
the 'prototype' argument was in the middle then it would get lost in all the
other arguments and be less readable as a prototype.
> >
> > STUB_UNLESS(CONFIG_CPU_FREQ,
> > struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu), return NULL);
> >
> > or:
> >
> > STUB_UNLESS(CONFIG_CPU_FREQ,
> > struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu),
> > return NULL);
As you indicate here, it's possible to spread this to three lines and keep the
readability of the prototype - though I was keen to condense it to as few lines
as possible (I was probably putting too much focus on the diff stat).
> >
> > Seems to be more readable in terms of the flow.
>
> Hmmm, looking at that, I probably prefer that too.
Feedback I've had so far suggests that there is a preference to putting the
optional argument at the end, I have no objection to this.
>
> In the unlikely case that <body> uses the function arguments it would be
> quite confusing to have the body before the function prototype.
>
> If we can keep this down to two lines so much the better, but still
> seems fine.
This is the compromise - having the optional argument after the prototype
will likely result in wrapping to the next line as prototypes tend to be
long. Perhaps this is more readable.
>
> Provided we don't end up needing a trailing comma in the void case, to
> supply the empty body argument, that is.
No this isn't necessary.
Thanks,
Andrew Murray
>
> Cheers
> ---Dave