Re: [PATCH 1/2] pinctrl: remove unneeded #ifdef around declarations

[Masahiro Yamada]

On Tue, Jun 25, 2019 at 5:52 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> Hi Masahiro,
>
> thanks for your patch. For some reason I managed to pick up
> patch 2 before patch 1. I applied this now with some fuzzing.
> (Please check the result.)
>
> On Thu, Jun 13, 2019 at 3:55 AM Masahiro Yamada
> <yamada.masahiro@xxxxxxxxxxxxx> wrote:
>
> > What is the point in surrounding the whole of declarations with
> > ifdef like this?
>
> I don't know if it is generally good to have phrases posed as
> questions in a commit message, we prefer to have statements
> about the change not a polemic dialog.
>
> >   #ifdef CONFIG_FOO
> >   int foo(void);
> >   #endif
> >
> > If CONFIG_FOO is not defined, all callers of foo() will fail
> > with implicit declaration errors since the top Makefile adds
> > -Werror-implicit-function-declaration to KBUILD_CFLAGS.
>
> Maybe this flag was not in the top Makefile when the #ifdefs
> where introduced?
>
> > This breaks the build earlier when you are doing something wrong.
> > That's it.
>
> Good idea.
>
> > Anyway, it will fail to link since the definition of foo() is not
> > compiled.
> >
> > In summary, these ifdef are unneeded.
> >
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
>
> Pushing this to the zeroday builders and let's see what happens!


Sorry for my terrible commit message and offending response.

I appended a new commit message below.

If you are OK with rebasing, please consider replacement.

--------------------------->8----------------------------------
pinctrl: remove less important #ifdef around declarations

The whole declarations in these headers are surrounded by #ifdef.

As far as I understood, the motivation of this is probably to break
the build earlier if a driver misses to select or depend on correct
CONFIG options in Kconfig.

Since commit 94bed2a9c4ae ("Add -Werror-implicit-function-declaration")
no one cannot call functions that have not been declared.

So, I see some benefit in doing this in the cost of uglier headers.

In reality, it would not be so easy to catch missed 'select' or
'depends on' because PINCTRL, PINMUX, etc. are already selected by
someone else eventually. So, this kind of error, if any, will be
caught by randconfig bots.

In summary, I am not a big fan of deep #ifdef nesting, and this
does not matter for normal developers. The code readability wins.
--------------------------->8----------------------------------




-- 
Best Regards
Masahiro Yamada