format-security: move static strings to const
Subsystems (19)
PATCH 1/1 format-security: move static strings to const
While examining output from trial builds with -Wformat-security enabled, many strings were found that should be defined as "const", or as a char array instead of char pointer. This makes some static analysis easier, by producing fewer false positives.
As these are all trivial changes, it seemed best to put them all in a single patch rather than chopping them up per maintainer.
Signed-off-by Kees Cook <[email protected]>
--- arch/arm/mach-omap2/board-n8x0.c | 2 +- arch/mips/dec/prom/init.c | 6 +++--- arch/mips/kernel/traps.c | 4 ++-- drivers/char/dsp56k.c | 2 +- drivers/cpufreq/powernow-k8.c | 3 ++- drivers/gpu/drm/drm_fb_helper.c | 2 +- drivers/net/ethernet/amd/atarilance.c | 4 ++-- drivers/net/ethernet/amd/declance.c | 2 +- drivers/net/ethernet/amd/sun3lance.c | 3 ++- drivers/net/ethernet/cirrus/mac89x0.c | 2 +- drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 2 +- drivers/net/ethernet/natsemi/sonic.h | 2 +- drivers/net/ethernet/toshiba/tc35815.c | 2 +- drivers/net/fddi/defxx.c | 2 +- drivers/net/hippi/rrunner.c | 3 ++- drivers/staging/most/mostcore/core.c | 2 +- drivers/tty/n_hdlc.c | 10 +++++----- drivers/tty/serial/st-asc.c | 2 +- net/decnet/af_decnet.c | 3 ++- 19 files changed, 31 insertions(+), 27 deletions(-)
arch/arm/mach-omap2/board-n8x0.c +1 -1
@@ -117,7 +117,7 @@ static struct musb_hdrc_platform_data tusb_data = {static void __init n8x0_usb_init (void ) {int ret =0 ; -static char announce[]__initdata =KERN_INFO "TUSB 6010\n" ; +static const char announce[]__initconst =KERN_INFO "TUSB 6010\n" ;/* PM companion chip power control pin */ ret = gpio_request_one(TUSB6010_GPIO_ENABLE, GPIOF_OUT_INIT_LOW,
arch/mips/dec/prom/init.c +3 -3
@@ -88,7 +88,7 @@ void __init which_prom(s32 magic, s32 *prom_vec)void __init prom_init (void ) {extern void dec_machine_halt (void ); -static char cpu_msg[]__initdata = +static const char cpu_msg[]__initconst ="Sorry, this kernel is compiled for a wrong CPU type!\n" ;s32 argc = fw_arg0;s32 *argv = (void *)fw_arg1;
@@ -111,7 +111,7 @@ void __init prom_init(void)#if defined(CONFIG_CPU_R3000 )if ((current_cpu_type () ==CPU_R4000SC ) || (current_cpu_type () ==CPU_R4400SC )) { -static char r4k_msg[]__initdata = +static const char r4k_msg[]__initconst ="Please recompile with \"CONFIG_CPU_R4x00 = y\".\n" ;printk (cpu_msg);printk (r4k_msg);
@@ -122,7 +122,7 @@ void __init prom_init(void)#if defined(CONFIG_CPU_R4X00 )if ((current_cpu_type () ==CPU_R3000 ) || (current_cpu_type () ==CPU_R3000A )) { -static char r3k_msg[]__initdata = +static const char r3k_msg[]__initconst ="Please recompile with \"CONFIG_CPU_R3000 = y\".\n" ;printk (cpu_msg);printk (r3k_msg);
arch/mips/kernel/traps.c +2 -2
@@ -2256,8 +2256,8 @@ void set_handler(unsigned long offset, void *addr, unsigned long size)local_flush_icache_range (ebase + offset, ebase + offset + size); } -static char panic_null_cerr[] = -"Trying to set NULL cache error exception handler" ; +static const char panic_null_cerr[] = +"Trying to set NULL cache error exception handler\n" ;/* * Install uncached CPU exception handler.
drivers/char/dsp56k.c +1 -1
@@ -489,7 +489,7 @@ static const struct file_operations dsp56k_fops = {/****** Init and module functions ******/ -static char banner[]__initdata =KERN_INFO "DSP56k driver installed\n" ; +static const char banner[]__initconst =KERN_INFO "DSP56k driver installed\n" ;static int __init dsp56k_init_driver (void ) {
drivers/cpufreq/powernow-k8.c +2 -1
@@ -1171,7 +1171,8 @@ static struct cpufreq_driver cpufreq_amd64_driver = {static void __request_acpi_cpufreq (void ) { -const char *cur_drv, *drv ="acpi-cpufreq" ; +const char drv[] ="acpi-cpufreq" ; +const char *cur_drv; cur_drv =cpufreq_get_current_driver ();if (!cur_drv)
drivers/gpu/drm/drm_fb_helper.c +1 -1
@@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);int __init drm_fb_helper_modinit (void ) {#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE ) && !defined(CONFIG_EXPERT ) -const char *name ="fbcon" ; +const char name[] ="fbcon" ;struct module *fbcon;mutex_lock (&module_mutex);
drivers/net/ethernet/amd/atarilance.c +2 -2
@@ -42,8 +42,8 @@*/ -static char version[] ="atarilance.c: v1.3 04/04/96 " -"[email protected]\n" ; +static const char version[] ="atarilance.c: v1.3 04/04/96 " +"[email protected]\n" ;#include <linux/netdevice.h> #include <linux/etherdevice.h>
drivers/net/ethernet/amd/declance.c +1 -1
@@ -72,7 +72,7 @@#include <asm/dec/machtype.h> #include <asm/dec/system.h> -static char version[] = +static const char version[] ="declance.c: v0.011 by Linux MIPS DECstation task force\n" ;MODULE_AUTHOR ("Linux MIPS DECstation task force" );
drivers/net/ethernet/amd/sun3lance.c +2 -1
@@ -21,7 +21,8 @@*/ -static char *version ="sun3lance.c: v1.2 1/12/2001 Sam Creasey ([email protected])\n" ; +static const char version[] = +"sun3lance.c: v1.2 1/12/2001 Sam Creasey ([email protected])\n" ;#include <linux/module.h> #include <linux/stddef.h>
drivers/net/ethernet/cirrus/mac89x0.c +1 -1
@@ -56,7 +56,7 @@local_irq_{dis,en}able() */ -static char *version = +static const char version[] ="cs89x0.c:v1.02 11/26/96 Russell Nelson <[email protected]>\n" ;/* ======================= configure the driver here ======================= */
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h +1 -1
@@ -407,7 +407,7 @@ struct mac_driver { };struct mac_stats_string { -char desc[ETH_GSTRING_LEN ]; +const char desc[ETH_GSTRING_LEN ];unsigned long offset; };
drivers/net/ethernet/natsemi/sonic.h +1 -1
@@ -444,7 +444,7 @@ static inline __u16 sonic_rra_get(struct net_device* dev, int entry, (entry * SIZEOF_SONIC_RR) + offset); } -static const char *version = +static const char version[] ="sonic.c:v0.92 20.9.98 [email protected]\n" ;#endif /* SONIC_H */
drivers/net/ethernet/toshiba/tc35815.c +1 -1
@@ -23,7 +23,7 @@*/ #define DRV_VERSION "1.39" -static const char *version ="tc35815.c:v" DRV_VERSION "\n" ; +static const char version[] ="tc35815.c:v" DRV_VERSION "\n" ;#define MODNAME "tc35815"#include <linux/module.h>
drivers/net/fddi/defxx.c +1 -1
@@ -228,7 +228,7 @@#define DRV_VERSION "v1.11"#define DRV_RELDATE "2014/07/01" -static char version[] = +static const char version[] =DRV_NAME ": " DRV_VERSION " " DRV_RELDATE " Lawrence V. Stefani and others\n" ;
drivers/net/hippi/rrunner.c +2 -1
@@ -60,7 +60,8 @@ MODULE_AUTHOR("Jes Sorensen <[email protected]>");MODULE_DESCRIPTION ("Essential RoadRunner HIPPI driver" );MODULE_LICENSE ("GPL" ); -static char version[] ="rrunner.c: v0.50 11/11/2002 Jes Sorensen ([email protected])\n" ; +static const char version[] = +"rrunner.c: v0.50 11/11/2002 Jes Sorensen ([email protected])\n" ;static const struct net_device_ops rr_netdev_ops = {
drivers/staging/most/mostcore/core.c +1 -1
@@ -82,7 +82,7 @@ struct most_inst_obj {static const struct {int most_ch_data_type; -char *name; +const char *name; } ch_data_type[] = { {MOST_CH_CONTROL ,"control\n" }, {MOST_CH_ASYNC ,"async\n" },
drivers/tty/n_hdlc.c +5 -5
@@ -939,11 +939,11 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)return buf; }/* end of n_hdlc_buf_get() */ -static char hdlc_banner[]__initdata = +static const char hdlc_banner[]__initconst =KERN_INFO "HDLC line discipline maxframe=%u\n" ; -static char hdlc_register_ok[]__initdata = +static const char hdlc_register_ok[]__initconst =KERN_INFO "N_HDLC line discipline registered.\n" ; -static char hdlc_register_fail[]__initdata = +static const char hdlc_register_fail[]__initconst =KERN_ERR "error registering line discipline: %d\n" ;static int __init n_hdlc_init (void )
@@ -968,9 +968,9 @@ static int __init n_hdlc_init(void) }/* end of init_module() */ -static char hdlc_unregister_ok[] __exitdata = +static const char hdlc_unregister_ok[] __exitdata =KERN_INFO "N_HDLC: line discipline unregistered\n" ; -static char hdlc_unregister_fail[] __exitdata = +static const char hdlc_unregister_fail[] __exitdata =KERN_ERR "N_HDLC: can't unregister line discipline (err = %d)\n" ; static void __exit n_hdlc_exit(void)
drivers/tty/serial/st-asc.c +1 -1
@@ -985,7 +985,7 @@ static struct platform_driver asc_serial_driver = {static int __init asc_init (void ) {int ret; -static char banner[]__initdata = +static const char banner[]__initconst =KERN_INFO "STMicroelectronics ASC driver initialized\n" ;printk (banner);
net/decnet/af_decnet.c +2 -1
@@ -2359,7 +2359,8 @@ MODULE_AUTHOR("Linux DECnet Project Team");MODULE_LICENSE ("GPL" );MODULE_ALIAS_NETPROTO (PF_DECnet); -static char banner[]__initdata =KERN_INFO "NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n" ; +static const char banner[]__initconst =KERN_INFO +"NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n" ;static int __init decnet_init (void ) {
Signature · 6 lines
2.7.4 -- Kees Cook Pixel Security
Quoted 9 lines · Kees Cook
While examining output from trial builds with -Wformat-security enabled, many strings were found that should be defined as "const", or as a char array instead of char pointer. This makes some static analysis easier, by producing fewer false positives.
As these are all trivial changes, it seemed best to put them all in a single patch rather than chopping them up per maintainer.
Signed-off-by Kees Cook <[email protected]>
Ack on the drm part.
-Daniel
Quoted 324 lines
--- arch/arm/mach-omap2/board-n8x0.c | 2 +- arch/mips/dec/prom/init.c | 6 +++--- arch/mips/kernel/traps.c | 4 ++-- drivers/char/dsp56k.c | 2 +- drivers/cpufreq/powernow-k8.c | 3 ++- drivers/gpu/drm/drm_fb_helper.c | 2 +- drivers/net/ethernet/amd/atarilance.c | 4 ++-- drivers/net/ethernet/amd/declance.c | 2 +- drivers/net/ethernet/amd/sun3lance.c | 3 ++- drivers/net/ethernet/cirrus/mac89x0.c | 2 +- drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 2 +- drivers/net/ethernet/natsemi/sonic.h | 2 +- drivers/net/ethernet/toshiba/tc35815.c | 2 +- drivers/net/fddi/defxx.c | 2 +- drivers/net/hippi/rrunner.c | 3 ++- drivers/staging/most/mostcore/core.c | 2 +- drivers/tty/n_hdlc.c | 10 +++++----- drivers/tty/serial/st-asc.c | 2 +- net/decnet/af_decnet.c | 3 ++- 19 files changed, 31 insertions(+), 27 deletions(-)arch/arm/mach-omap2/board-n8x0.c +1
-1@@ -117,7 +117,7 @@ static struct musb_hdrc_platform_data tusb_data = {static void __init n8x0_usb_init (void ) {int ret =0 ; -static char announce[]__initdata =KERN_INFO "TUSB 6010\n" ; +static const char announce[]__initconst =KERN_INFO "TUSB 6010\n" ;/* PM companion chip power control pin */ ret = gpio_request_one(TUSB6010_GPIO_ENABLE, GPIOF_OUT_INIT_LOW,arch/mips/dec/prom/init.c +3
-3@@ -88,7 +88,7 @@ void __init which_prom(s32 magic, s32 *prom_vec)void __init prom_init (void ) {extern void dec_machine_halt (void ); -static char cpu_msg[]__initdata = +static const char cpu_msg[]__initconst ="Sorry, this kernel is compiled for a wrong CPU type!\n" ;s32 argc = fw_arg0;s32 *argv = (void *)fw_arg1;@@ -111,7 +111,7 @@ void __init prom_init(void)#if defined(CONFIG_CPU_R3000 )if ((current_cpu_type () ==CPU_R4000SC ) || (current_cpu_type () ==CPU_R4400SC )) { -static char r4k_msg[]__initdata = +static const char r4k_msg[]__initconst ="Please recompile with \"CONFIG_CPU_R4x00 = y\".\n" ;printk (cpu_msg);printk (r4k_msg);@@ -122,7 +122,7 @@ void __init prom_init(void)#if defined(CONFIG_CPU_R4X00 )if ((current_cpu_type () ==CPU_R3000 ) || (current_cpu_type () ==CPU_R3000A )) { -static char r3k_msg[]__initdata = +static const char r3k_msg[]__initconst ="Please recompile with \"CONFIG_CPU_R3000 = y\".\n" ;printk (cpu_msg);printk (r3k_msg);arch/mips/kernel/traps.c +2
-2@@ -2256,8 +2256,8 @@ void set_handler(unsigned long offset, void *addr, unsigned long size)local_flush_icache_range (ebase + offset, ebase + offset + size); } -static char panic_null_cerr[] = -"Trying to set NULL cache error exception handler" ; +static const char panic_null_cerr[] = +"Trying to set NULL cache error exception handler\n" ;/* * Install uncached CPU exception handler. drivers/char/dsp56k.c +1
-1@@ -489,7 +489,7 @@ static const struct file_operations dsp56k_fops = {/****** Init and module functions ******/ -static char banner[]__initdata =KERN_INFO "DSP56k driver installed\n" ; +static const char banner[]__initconst =KERN_INFO "DSP56k driver installed\n" ;static int __init dsp56k_init_driver (void ) {drivers/cpufreq/powernow-k8.c +2
-1@@ -1171,7 +1171,8 @@ static struct cpufreq_driver cpufreq_amd64_driver = {static void __request_acpi_cpufreq (void ) { -const char *cur_drv, *drv ="acpi-cpufreq" ; +const char drv[] ="acpi-cpufreq" ; +const char *cur_drv; cur_drv =cpufreq_get_current_driver ();if (!cur_drv)drivers/gpu/drm/drm_fb_helper.c +1
-1@@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);int __init drm_fb_helper_modinit (void ) {#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE ) && !defined(CONFIG_EXPERT ) -const char *name ="fbcon" ; +const char name[] ="fbcon" ;struct module *fbcon;mutex_lock (&module_mutex);drivers/net/ethernet/amd/atarilance.c +2
-2@@ -42,8 +42,8 @@*/ -static char version[] ="atarilance.c: v1.3 04/04/96 " -"[email protected]\n" ; +static const char version[] ="atarilance.c: v1.3 04/04/96 " +"[email protected]\n" ;#include <linux/netdevice.h> #include <linux/etherdevice.h> drivers/net/ethernet/amd/declance.c +1
-1@@ -72,7 +72,7 @@#include <asm/dec/machtype.h> #include <asm/dec/system.h> -static char version[] = +static const char version[] ="declance.c: v0.011 by Linux MIPS DECstation task force\n" ;MODULE_AUTHOR ("Linux MIPS DECstation task force" );drivers/net/ethernet/amd/sun3lance.c +2
-1@@ -21,7 +21,8 @@*/ -static char *version ="sun3lance.c: v1.2 1/12/2001 Sam Creasey ([email protected])\n" ; +static const char version[] = +"sun3lance.c: v1.2 1/12/2001 Sam Creasey ([email protected])\n" ;#include <linux/module.h> #include <linux/stddef.h> drivers/net/ethernet/cirrus/mac89x0.c +1
-1@@ -56,7 +56,7 @@local_irq_{dis,en}able() */ -static char *version = +static const char version[] ="cs89x0.c:v1.02 11/26/96 Russell Nelson <[email protected]>\n" ;/* ======================= configure the driver here ======================= */ drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h +1
-1@@ -407,7 +407,7 @@ struct mac_driver { };struct mac_stats_string { -char desc[ETH_GSTRING_LEN ]; +const char desc[ETH_GSTRING_LEN ];unsigned long offset; };drivers/net/ethernet/natsemi/sonic.h +1
-1@@ -444,7 +444,7 @@ static inline __u16 sonic_rra_get(struct net_device* dev, int entry, (entry * SIZEOF_SONIC_RR) + offset); } -static const char *version = +static const char version[] ="sonic.c:v0.92 20.9.98 [email protected]\n" ;#endif /* SONIC_H */ drivers/net/ethernet/toshiba/tc35815.c +1
-1@@ -23,7 +23,7 @@*/ #define DRV_VERSION "1.39" -static const char *version ="tc35815.c:v" DRV_VERSION "\n" ; +static const char version[] ="tc35815.c:v" DRV_VERSION "\n" ;#define MODNAME "tc35815"#include <linux/module.h> drivers/net/fddi/defxx.c +1
-1@@ -228,7 +228,7 @@#define DRV_VERSION "v1.11"#define DRV_RELDATE "2014/07/01" -static char version[] = +static const char version[] =DRV_NAME ": " DRV_VERSION " " DRV_RELDATE " Lawrence V. Stefani and others\n" ;drivers/net/hippi/rrunner.c +2
-1@@ -60,7 +60,8 @@ MODULE_AUTHOR("Jes Sorensen <[email protected]>");MODULE_DESCRIPTION ("Essential RoadRunner HIPPI driver" );MODULE_LICENSE ("GPL" ); -static char version[] ="rrunner.c: v0.50 11/11/2002 Jes Sorensen ([email protected])\n" ; +static const char version[] = +"rrunner.c: v0.50 11/11/2002 Jes Sorensen ([email protected])\n" ;static const struct net_device_ops rr_netdev_ops = {drivers/staging/most/mostcore/core.c +1
-1@@ -82,7 +82,7 @@ struct most_inst_obj {static const struct {int most_ch_data_type; -char *name; +const char *name; } ch_data_type[] = { {MOST_CH_CONTROL ,"control\n" }, {MOST_CH_ASYNC ,"async\n" },drivers/tty/n_hdlc.c +5
-5@@ -939,11 +939,11 @@ static struct n_hdlc_buf *n_hdlc_buf_get(struct n_hdlc_buf_list *buf_list)return buf; }/* end of n_hdlc_buf_get() */ -static char hdlc_banner[]__initdata = +static const char hdlc_banner[]__initconst =KERN_INFO "HDLC line discipline maxframe=%u\n" ; -static char hdlc_register_ok[]__initdata = +static const char hdlc_register_ok[]__initconst =KERN_INFO "N_HDLC line discipline registered.\n" ; -static char hdlc_register_fail[]__initdata = +static const char hdlc_register_fail[]__initconst =KERN_ERR "error registering line discipline: %d\n" ;static int __init n_hdlc_init (void )@@ -968,9 +968,9 @@ static int __init n_hdlc_init(void) }/* end of init_module() */ -static char hdlc_unregister_ok[] __exitdata = +static const char hdlc_unregister_ok[] __exitdata =KERN_INFO "N_HDLC: line discipline unregistered\n" ; -static char hdlc_unregister_fail[] __exitdata = +static const char hdlc_unregister_fail[] __exitdata =KERN_ERR "N_HDLC: can't unregister line discipline (err = %d)\n" ; static void __exit n_hdlc_exit(void)drivers/tty/serial/st-asc.c +1
-1@@ -985,7 +985,7 @@ static struct platform_driver asc_serial_driver = {static int __init asc_init (void ) {int ret; -static char banner[]__initdata = +static const char banner[]__initconst =KERN_INFO "STMicroelectronics ASC driver initialized\n" ;printk (banner);net/decnet/af_decnet.c +2
-1@@ -2359,7 +2359,8 @@ MODULE_AUTHOR("Linux DECnet Project Team");MODULE_LICENSE ("GPL" );MODULE_ALIAS_NETPROTO (PF_DECnet); -static char banner[]__initdata =KERN_INFO "NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n" ; +static const char banner[]__initconst =KERN_INFO +"NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n" ;static int __init decnet_init (void ) {Signature · 10 lines
2.7.4 -- Kees Cook Pixel Security _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 13 lines · Kees Cook
While examining output from trial builds with -Wformat-security enabled, many strings were found that should be defined as "const", or as a char array instead of char pointer. This makes some static analysis easier, by producing fewer false positives.
As these are all trivial changes, it seemed best to put them all in a single patch rather than chopping them up per maintainer.
drivers/gpu/drm/drm_fb_helper.c +0
-0
drivers/gpu/drm/drm_fb_helper.c EXPORT_SYMBOL(drm_fb_helper_hotplug_event); · line 2334
Quoted 5 lines
drivers/gpu/drm/drm_fb_helper.c +1
-1@@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);int __init drm_fb_helper_modinit (void ) {#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE ) && !defined(CONFIG_EXPERT ) -const char *name ="fbcon" ; +const char name[] ="fbcon" ;
I'd always write the former out of habit. Why should I start using the latter? What makes it better?
What keeps the kernel from accumulating tons more of the former?
Here's an interesting comparison of the generated code. I'm a bit surprised by what gcc does, I would have expected no difference, like clang. https://godbolt.org/g/OdqUvN
The other changes adding const in this patch are, of course, good.
BR,
Jani.
Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 9 lines · Jani Nikula
Quoted 7 lines · Kees Cook
While examining output from trial builds with -Wformat-security enabled, many strings were found that should be defined as "const", or as a char array instead of char pointer. This makes some static analysis easier, by producing fewer false positives.
As these are all trivial changes, it seemed best to put them all in a single patch rather than chopping them up per maintainer.
Quoted 13 lines
drivers/gpu/drm/drm_fb_helper.c EXPORT_SYMBOL(drm_fb_helper_hotplug_event); · line 2334
Quoted 10 lines
drivers/gpu/drm/drm_fb_helper.c +1
-1@@ -2331,7 +2331,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);int __init drm_fb_helper_modinit (void ) {#if defined(CONFIG_FRAMEBUFFER_CONSOLE_MODULE ) && !defined(CONFIG_EXPERT ) -const char *name ="fbcon" ; +const char name[] ="fbcon" ;I'd always write the former out of habit. Why should I start using the latter? What makes it better?
For me, it's mainly two reasons: sizeof() and -Wformat-security behavior.
The compiler treats "sizeof" differently. E.g. "sizeof(var)" shows the allocation size for the array, and pointer size for the char pointer. When doing things like snprintf(buf, sizeof(buf), ...) will do the right thing, etc. (This is a poor example for a _const_ string, but the point is that some calculations still work better with the array over the pointer.)
The other situation (which is why I noted this to change them) is that gcc's handling of them is different when faced with -Wformat-security since it doesn't like to believe that const char pointers are actually const for the purposes of being a format string.
Quoted 1 line
What keeps the kernel from accumulating tons more of the former?
Right now, nothing. The good news is that they're relatively rare, and I notice them when they're added (since I have a -Wformat-security tree). We could add a warning to checkpatch for suggesting const char var[] over const char *var, perhaps?
Quoted 3 lines
Here's an interesting comparison of the generated code. I'm a bit surprised by what gcc does, I would have expected no difference, like clang. https://godbolt.org/g/OdqUvN
Here's your example with sizeof() added, if you're curious...
https://godbolt.org/g/U1zIZK
Quoted 1 line
The other changes adding const in this patch are, of course, good.
Thanks!
-Kees
Signature · 2 lines
Kees Cook Pixel Security
Quoted 22 lines · Kees Cook
While examining output from trial builds with -Wformat-security enabled, many strings were found that should be defined as "const", or as a char array instead of char pointer. This makes some static analysis easier, by producing fewer false positives.
As these are all trivial changes, it seemed best to put them all in a single patch rather than chopping them up per maintainer.
Signed-off-by Kees Cook <[email protected]>--- arch/arm/mach-omap2/board-n8x0.c | 2 +- arch/mips/dec/prom/init.c | 6 +++--- arch/mips/kernel/traps.c | 4 ++-- drivers/char/dsp56k.c | 2 +- drivers/cpufreq/powernow-k8.c | 3 ++- drivers/gpu/drm/drm_fb_helper.c | 2 +- drivers/net/ethernet/amd/atarilance.c | 4 ++-- drivers/net/ethernet/amd/declance.c | 2 +- drivers/net/ethernet/amd/sun3lance.c | 3 ++- drivers/net/ethernet/cirrus/mac89x0.c | 2 +- drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.h | 2 +- drivers/net/ethernet/natsemi/sonic.h | 2 +-
Quoted 8 lines
drivers/net/ethernet/toshiba/tc35815.c | 2 +- drivers/net/fddi/defxx.c | 2 +- drivers/net/hippi/rrunner.c | 3 ++- drivers/staging/most/mostcore/core.c | 2 +- drivers/tty/n_hdlc.c | 10 +++++----- drivers/tty/serial/st-asc.c | 2 +- net/decnet/af_decnet.c | 3 ++- 19 files changed, 31 insertions(+), 27 deletions(-)
rrunner.c changes look fine to me.
Jes