All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Kaiwan N Billimoria <kaiwan@kaiwantech.com>
Cc: Kees Cook <keescook@chromium.org>,
	Laura Abbott <labbott@redhat.com>,
	"kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>,
	nd@arm.com
Subject: Re: [kernel-hardening] Merge in PAX_MEMORY_SANITIZE work from grsec to linux-next
Date: Tue, 14 Feb 2017 16:19:03 +0000	[thread overview]
Message-ID: <20170214161902.GH23718@leverpostej> (raw)
In-Reply-To: <CAPDLWs8iS_xU0uQHKU1h6HmqP-oqAfK1qAxDpis_JyPSGVGLug@mail.gmail.com>

On Tue, Feb 14, 2017 at 01:34:38PM +0530, Kaiwan N Billimoria wrote:
> >> diff --git a/mm/page_poison.c b/mm/page_poison.c
> >> index 2e647c6..b45bc0a 100644
> >> --- a/mm/page_poison.c
> >> +++ b/mm/page_poison.c
> >> @@ -49,6 +49,19 @@ struct page_ext_operations page_poisoning_ops = {
> >>          .init = init_page_poisoning,
> >>  };
> >>
> >> +#ifdef CONFIG_MEMORY_SANITIZE
> >> +static int __init memory_sanitize_init(void)
> >> +{
> >> +    /* With 'memory sanitize' On, page poisoning Must be turned on */
> >> +    if (IS_ENABLED(CONFIG_MEMORY_SANITIZE)) {
> >> +        want_page_poisoning = true;
> >> +        __page_poisoning_enabled = true;
> >> +    }
> >> +    return 0;
> >> +}
> >> +early_initcall(memory_sanitize_init);
> >> +#endif
> >
> > The ifdef and the IS_ENABLED seem redundant to me. I'd drop the ifdef.
> 
> An academic question perhaps- am not clear as to why the IS_ENABLED()
> is preferable to an #ifdef. I thought eliminating a runtime 'if'
> condition (by using an ifdef) is more optimal than the "if
> IS_ENABLED(foo)"? Or (probably) does the compiler optimize it out
> regardless (assuming that CONFIG_MEMORY_SANITIZE is not selected of
> course)?

We expect the compiler to optimize the code out, since IS_ENABLED(x) is
a compile-time constant.

As for why it's preferable, it has several benefits.

For one thing, using if (IS_ENABLED(x)) means that we always get build
coverage of the code predicated on the config option, so it's harder to
accidentally break some build configurations.

For another, it composes more nicely with other conditionals, which is
useful when you have a condition that depends on several config options,
or config options and runtime expressions.

e.g. something like:

	if (IS_ENABLED(FOO) && IS_ENABLED(BAR) && runtime_option)
		do_combination_thing();
	else if (IS_ENABLED(BAR))
		do_bar_only_thing();
	else
		do_other_thing();

... is much more painful to write using ifdefs.

Generally, it's nicer for people to deal with than ifdeffery. It's
easier for people to match braces in well-formatted code than it is to
match #ifdef #endif pairs.

Thanks,
Mark.

  reply	other threads:[~2017-02-14 16:19 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-18  4:21 [kernel-hardening] Merge in PAX_MEMORY_SANITIZE work from grsec to linux-next Kaiwan N Billimoria
2017-01-18 19:44 ` Laura Abbott
2017-01-31  1:49   ` Kaiwan N Billimoria
2017-02-03  4:49   ` Kaiwan N Billimoria
2017-02-04  0:23     ` Kees Cook
2017-02-04  3:12       ` Kaiwan N Billimoria
2017-02-09  3:37       ` Kaiwan N Billimoria
2017-02-13 11:42         ` Kaiwan N Billimoria
2017-02-13 17:28         ` Kees Cook
2017-02-14  3:01           ` Kaiwan N Billimoria
2017-02-14 17:19             ` Kees Cook
2017-02-15  7:27               ` Kaiwan N Billimoria
2017-02-15 20:30                 ` Kees Cook
2017-02-17  3:18                   ` Kaiwan N Billimoria
2017-02-21 23:26                     ` Kees Cook
2017-02-24  9:38                       ` Kaiwan N Billimoria
2017-02-14  8:04           ` Kaiwan N Billimoria
2017-02-14 16:19             ` Mark Rutland [this message]
2017-02-14 16:33               ` Kaiwan N Billimoria
2017-01-19  0:54 ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170214161902.GH23718@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=kaiwan@kaiwantech.com \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=labbott@redhat.com \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.