Re: module: Remove const attribute from alias for MODULE_DEVICE_TABLE

From: Stefan Agner
Date: Sun Aug 27 2017 - 19:59:57 EST


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)