| Message ID | 1404911056-29064-4-git-send-email-elder@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 09, 2014 at 08:04:15AM -0500, Alex Elder wrote: > Use the IS_ENABLED() macro rather than #ifdef blocks to set certain > global values. > > Signed-off-by: Alex Elder <elder@linaro.org> That's good - every patch which removes ifdeffery is a good patch. :) Acked-by: Borislav Petkov <bp@suse.de>
On Wed 2014-07-09 08:04:15, Alex Elder wrote: > Use the IS_ENABLED() macro rather than #ifdef blocks to set certain > global values. > > Signed-off-by: Alex Elder <elder@linaro.org> > --- > kernel/printk/printk.c | 12 ++---------- > 1 file changed, 2 insertions(+), 10 deletions(-) > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index 646146c..6f75e8a 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -453,11 +453,7 @@ static int log_store(int facility, int level, > return msg->text_len; > } > > -#ifdef CONFIG_SECURITY_DMESG_RESTRICT > -int dmesg_restrict = 1; > -#else > -int dmesg_restrict; > -#endif > +int dmesg_restrict = IS_ENABLED(CONFIG_SECURITY_DMESG_RESTRICT); I wondered if IS_ENABLED() is guaranteed to set 1 when enabled. Well, it does not matter because the variable is used as a boolean. > static int syslog_action_restricted(int type) > { > @@ -947,11 +943,7 @@ static inline void boot_delay_msec(int level) > } > #endif > > -#if defined(CONFIG_PRINTK_TIME) > -static bool printk_time = 1; > -#else > -static bool printk_time; > -#endif > +static bool printk_time = IS_ENABLED(CONFIG_PRINTK_TIME); > module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR); > > static size_t print_time(u64 ts, char *buf) > -- > 1.9.1 Nice clean up. Reviewed-by: Petr Mladek <pmladek@suse.cz> Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Wed, Jul 09, 2014 at 06:19:46PM +0200, Petr Mládek wrote:
> I wondered if IS_ENABLED() is guaranteed to set 1 when enabled.
See include/linux/kconfig.h
On Wed 2014-07-09 18:24:04, Borislav Petkov wrote: > On Wed, Jul 09, 2014 at 06:19:46PM +0200, Petr Mládek wrote: > > I wondered if IS_ENABLED() is guaranteed to set 1 when enabled. > > See include/linux/kconfig.h I know that it sets 1 now. I wondered about possible future changes. Well, it is probably too paranoid. Anyway, any non-zero value is fine. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Wed, Jul 09, 2014 at 06:32:05PM +0200, Petr Mládek wrote: > I know that it sets 1 now. I wondered about possible future changes. > Well, it is probably too paranoid. Anyway, any non-zero value is fine. Yes, because they both are used only in a boolean context.
Hi Alex, On Wed, Jul 9, 2014 at 3:04 PM, Alex Elder <elder@linaro.org> wrote: > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -453,11 +453,7 @@ static int log_store(int facility, int level, > return msg->text_len; > } > > -#ifdef CONFIG_SECURITY_DMESG_RESTRICT > -int dmesg_restrict = 1; > -#else > -int dmesg_restrict; > -#endif > +int dmesg_restrict = IS_ENABLED(CONFIG_SECURITY_DMESG_RESTRICT); Doesn't this move dmesg_restrict from the bss to the data section in case CONFIG_SECURITY_DMESG_RESTRICT is not enabled, due to the explicit initialization to zero? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 07/09/2014 12:58 PM, Geert Uytterhoeven wrote: > Hi Alex, > > On Wed, Jul 9, 2014 at 3:04 PM, Alex Elder <elder@linaro.org> wrote: >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -453,11 +453,7 @@ static int log_store(int facility, int level, >> return msg->text_len; >> } >> >> -#ifdef CONFIG_SECURITY_DMESG_RESTRICT >> -int dmesg_restrict = 1; >> -#else >> -int dmesg_restrict; >> -#endif >> +int dmesg_restrict = IS_ENABLED(CONFIG_SECURITY_DMESG_RESTRICT); > > Doesn't this move dmesg_restrict from the bss to the data section > in case CONFIG_SECURITY_DMESG_RESTRICT is not enabled, due > to the explicit initialization to zero? I honestly don't know. Is that even a well-defined behavior? Couldn't the compiler, noting an explicit 0 initialization, put it into BSS anyway? In any case, does this distinction matter? -Alex > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 646146c..6f75e8a 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -453,11 +453,7 @@ static int log_store(int facility, int level, return msg->text_len; } -#ifdef CONFIG_SECURITY_DMESG_RESTRICT -int dmesg_restrict = 1; -#else -int dmesg_restrict; -#endif +int dmesg_restrict = IS_ENABLED(CONFIG_SECURITY_DMESG_RESTRICT); static int syslog_action_restricted(int type) { @@ -947,11 +943,7 @@ static inline void boot_delay_msec(int level) } #endif -#if defined(CONFIG_PRINTK_TIME) -static bool printk_time = 1; -#else -static bool printk_time; -#endif +static bool printk_time = IS_ENABLED(CONFIG_PRINTK_TIME); module_param_named(time, printk_time, bool, S_IRUGO | S_IWUSR); static size_t print_time(u64 ts, char *buf)
Use the IS_ENABLED() macro rather than #ifdef blocks to set certain global values. Signed-off-by: Alex Elder <elder@linaro.org> --- kernel/printk/printk.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)