Re: module: Remove const attribute from alias for MODULE_DEVICE_TABLE

[Stefan Agner]

On 2017-07-24 18:27, Matthias Kaehlcke wrote:
> MODULE_DEVICE_TABLE(type, name) creates an alias of type 'extern const
> typeof(name)'. If 'name' is already constant the 'const' attribute is
> specified twice, which is not allowed in C89 (see discussion at
> https://lkml.org/lkml/2017/5/23/1440). Since the kernel is built with
> -std=gnu89 clang generates warnings like this:
> 
> drivers/thermal/x86_pkg_temp_thermal.c:509:1: warning: duplicate 'const'
>   declaration specifier
>       [-Wduplicate-decl-specifier]
> MODULE_DEVICE_TABLE(x86cpu, pkg_temp_thermal_ids);
> ^
> ./include/linux/module.h:212:8: note: expanded from macro 'MODULE_DEVICE_TABLE'
> extern const typeof(name) __mod_##type##__##name##_device_table
> 
> Remove the const attribute from the alias to avoid the duplicate
> specifier. After all it is only an alias and the attribute shouldn't
> have any effect.

Unfortunately, it has effect where const is missing in the original
variable declaration:

Before this patch:
13:10 $ size drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko
   text    data     bss     dec     hex filename
   8825     728      40    9593    2579
drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko

After this patch:
13:12 $ size drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko
   text    data     bss     dec     hex filename
   8747     800      40    9587    2573
drivers/net/wireless/ath/ath6kl/ath6kl_usb.ko


Ideally we would fix all MODULE_DEVICE_TABLE usage sites. This would
also made it clearly visible that the device tables are const.

I created a semantic patch, it turns out that 620 sites are affected
(out of 4499)...

//
// Cocinelle Semantic Patch to constify module device tables
//
// Author: Stefan Agner <stefan@xxxxxxxx>
//
@ module_device_table @
declarer name MODULE_DEVICE_TABLE;
identifier moduletype;
identifier name;
@@
MODULE_DEVICE_TABLE(moduletype, name);

@ add_const depends on module_device_table disable optional_qualifier @
identifier module_device_table.name;
type T;
@@
+const 
 T name[] = { 
 ...
 };

Thoughts?

--
Stefan


> 
> Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> ---
>  include/linux/module.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index e7bdd549e527..fe5aa3736707 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -209,7 +209,7 @@ extern void cleanup_module(void);
>  #ifdef MODULE
>  /* Creates an alias so file2alias.c can find device table. */
>  #define MODULE_DEVICE_TABLE(type, name)					\
> -extern const typeof(name) __mod_##type##__##name##_device_table		\
> +extern typeof(name) __mod_##type##__##name##_device_table		\
>    __attribute__ ((unused, alias(__stringify(name))))
>  #else  /* !MODULE */
>  #define MODULE_DEVICE_TABLE(type, name)