Linux: -Wunneeded-internal-declaration and MODULE_DEVICE_TABLE

[bulwahn]

There are a number of clang warnings of -Wunneeded-internal-declaration that are then passed on to MODULE_DEVICE_TABLE as argument.

So far, I am aware of two different situations:

1. ids passed to MODULE_DEVICE_TABLE. One example is:

  CC [M]  drivers/block/floppy.o
drivers/block/floppy.c:4970:35: warning: variable 'floppy_pnpids' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct pnp_device_id floppy_pnpids[] = {
                                  ^
1 warning generated.

Another example is #228 (that already has a patch to give a good example for the fix and explanation in the commit message)

or 2. the value is passed to MODULE_DEVICE_TABLE conditioned on the CONFIG_OF.

Here some random examples:

 CC [M]  sound/soc/codecs/alc5623.o
sound/soc/codecs/alc5623.c:1075:34: warning: variable 'alc5623_of_match' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct of_device_id alc5623_of_match[] = {
                                 ^
1 warning generated.

sound/soc/codecs/da7219.c:1583:34: warning: variable 'da7219_of_match' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct of_device_id da7219_of_match[] = {
                                 ^
1 warning generated.

Issue with MODULE_DEVICE_TABLE; special case with CONFIG_OF

sound/soc/codecs/max98090.c:2664:34: warning: variable 'max98090_of_match' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct of_device_id max98090_of_match[] = {
                                 ^
1 warning generated.

Issue with MODULE_DEVICE_TABLE; special case with CONFIG_OF

sound/soc/codecs/pcm179x-i2c.c:42:34: warning: variable 'pcm179x_of_match' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct of_device_id pcm179x_of_match[] = {
                                 ^
1 warning generated.

sound/soc/codecs/pcm179x-spi.c:41:34: warning: variable 'pcm179x_of_match' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct of_device_id pcm179x_of_match[] = {
                                 ^
1 warning generated.

  CC [M]  sound/soc/codecs/ts3a227e.o
sound/soc/codecs/ts3a227e.c:372:34: warning: variable 'ts3a227e_of_match' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const struct of_device_id ts3a227e_of_match[] = {
                                 ^
1 warning generated.

I believe in the first case, they should annotated with __used, in the second case with __maybe_used.
It would be nice if we could have a coccinelle script that checks (and refactors) that all incidents of variables passed to MODULE_DEVICE_TABLE are either of case 1 and annotated with __used or of case 2 and annotated with __maybe_used.

The issues #207, #205, #203, #178, #169 are candidates of the two cases above; I did not yet check for the individual issues whether they are case 1 or 2.

[bulwahn]

@tapasweni-pathak has some experience with coccinelle and she offered her assistance in writing coccinelle scripts for some of the refactorings we are identifying with the clang warnings.

[bulwahn]

@tapasweni-pathak if you need more information let us know.

[nathanchance]

Personally, I think we should try to come up with a better solution than __used/__maybe_used because nobody outside of us is going to remember to do that when they are creating new drivers, meaning that we may get all of these warnings fixed at one point but when new drivers get added, we'll need to fix up those as well. It's not a sustainable solution.

Stephen Boyd had an interesting proposal here. I don't know how that would look in practice but I think he's right in thinking that we need to target the MODULE_DEVICE_TABLE macro, rather than all of the declarations.

[bulwahn]

To the point of maintenance: if we create a coccinelle rule and get it upstream, coccinelle (through the 0day builds) would warn about wrong declarations.

But I do agree that Stephen's approach might be better if we can find out how such a macro should look like. For now, I would still propose to start to write a coccinelle script that can detect the syntactic pattern. Then in a next step, when we have that coccinelle rule, we can look into how to write the coccinelle rule such that it refactors it either in the new macro or adding annotations (depending on which path we then want to follow). At first, we do need a detecting cocci script, though.

[nickdesaulniers]

Or maybe I can make Clang not warn when referenced from an alias attribute?

[nathanchance]

Huh I never thought of that. That'd work out nicely if possible.

[nickdesaulniers]

The owner of Clang is cool with us fixing this in clang. I have a WIP patch fixing this. Will mark the other bugs as dups of this one later when I feel more confident that this is feasible.

[nickdesaulniers]

https://reviews.llvm.org/D54188

[dileks]

The patch from D54188 eliminates here 41 -Wunneeded-internal-declaration warnings in Linux v4.20-rc3.

[nickdesaulniers]

Fixed: https://reviews.llvm.org/rL350776