Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] security: Use IS_ENABLED() instead of checking for built-in or module

501 views
Skip to first unread message

Javier Martinez Canillas

unread,
Jul 14, 2016, 12:10:06 PM7/14/16
to
The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
built-in or as a module, use that macro instead of open coding the same.

Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
---

security/lsm_audit.c | 2 +-
security/selinux/hooks.c | 12 ++++++------
security/smack/smack_netfilter.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index cccbf3068cdc..5369036cf905 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -99,7 +99,7 @@ int ipv4_skb_to_auditdata(struct sk_buff *skb,
}
return ret;
}
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)
/**
* ipv6_skb_to_auditdata : fill auditdata from skb
* @skb : the skb
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index ec30880c4b98..c20ea9fe9274 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3984,7 +3984,7 @@ out:
return ret;
}

-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)

/* Returns error only if unable to parse addresses */
static int selinux_parse_skb_ipv6(struct sk_buff *skb,
@@ -4075,7 +4075,7 @@ static int selinux_parse_skb(struct sk_buff *skb, struct common_audit_data *ad,
&ad->u.net->v4info.daddr);
goto okay;

-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)
case PF_INET6:
ret = selinux_parse_skb_ipv6(skb, ad, proto);
if (ret)
@@ -5029,7 +5029,7 @@ static unsigned int selinux_ipv4_forward(void *priv,
return selinux_ip_forward(skb, state->in, PF_INET);
}

-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)
static unsigned int selinux_ipv6_forward(void *priv,
struct sk_buff *skb,
const struct nf_hook_state *state)
@@ -5087,7 +5087,7 @@ static unsigned int selinux_ipv4_output(void *priv,
return selinux_ip_output(skb, PF_INET);
}

-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)
static unsigned int selinux_ipv6_output(void *priv,
struct sk_buff *skb,
const struct nf_hook_state *state)
@@ -5273,7 +5273,7 @@ static unsigned int selinux_ipv4_postroute(void *priv,
return selinux_ip_postroute(skb, state->out, PF_INET);
}

-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)
static unsigned int selinux_ipv6_postroute(void *priv,
struct sk_buff *skb,
const struct nf_hook_state *state)
@@ -6317,7 +6317,7 @@ static struct nf_hook_ops selinux_nf_ops[] = {
.hooknum = NF_INET_LOCAL_OUT,
.priority = NF_IP_PRI_SELINUX_FIRST,
},
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)
{
.hook = selinux_ipv6_postroute,
.pf = NFPROTO_IPV6,
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index aa6bf1b22ec5..205b785fb400 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -20,7 +20,7 @@
#include <net/inet_sock.h>
#include "smack.h"

-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)

static unsigned int smack_ipv6_output(void *priv,
struct sk_buff *skb,
@@ -64,7 +64,7 @@ static struct nf_hook_ops smack_nf_ops[] = {
.hooknum = NF_INET_LOCAL_OUT,
.priority = NF_IP_PRI_SELINUX_FIRST,
},
-#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+#if IS_ENABLED(CONFIG_IPV6)
{
.hook = smack_ipv6_output,
.pf = NFPROTO_IPV6,
--
2.5.5

Casey Schaufler

unread,
Jul 14, 2016, 12:20:06 PM7/14/16
to
On 7/14/2016 9:00 AM, Javier Martinez Canillas wrote:
> The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
> built-in or as a module, use that macro instead of open coding the same.

Why?

Javier Martinez Canillas

unread,
Jul 14, 2016, 12:30:08 PM7/14/16
to
Hello Casey,

On 07/14/2016 12:17 PM, Casey Schaufler wrote:
> On 7/14/2016 9:00 AM, Javier Martinez Canillas wrote:
>> The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
>> built-in or as a module, use that macro instead of open coding the same.
>
> Why?
>

Why not? We have a macro for this so why is better to open coding it?

Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America

Casey Schaufler

unread,
Jul 14, 2016, 12:40:07 PM7/14/16
to
On 7/14/2016 9:20 AM, Javier Martinez Canillas wrote:
> Hello Casey,
>
> On 07/14/2016 12:17 PM, Casey Schaufler wrote:
>> On 7/14/2016 9:00 AM, Javier Martinez Canillas wrote:
>>> The IS_ENABLED() macro checks if a Kconfig symbol has been enabled either
>>> built-in or as a module, use that macro instead of open coding the same.
>> Why?
>>
> Why not? We have a macro for this so why is better to open coding it?

Unless there is a real advantage to IS_ENABLED() over ifdef there
is no value in making the change. Any change can introduce a problem,
so we don't make changes based on "why not". It's called code churn.


>
> Best regards,

Paul Moore

unread,
Jul 14, 2016, 4:00:05 PM7/14/16
to
I think the IS_ENABLED() macro makes the code more readable by helping
abstract away some of the Kconfig/module details; not to mention it
provides some insulation from Kconfig changes (although I suppose it
is doubtful this will be a real issue anytime soon).

Javier, if you want to respin this patch without the Smack changes
I'll merge it into the SELinux tree (not for the v4.8 merge window,
but for the next merge window). However, if Casey changes his mind
and ACKs this patch, I'll go ahead and merge the original patch.

--
paul moore
www.paul-moore.com

Casey Schaufler

unread,
Jul 14, 2016, 5:00:07 PM7/14/16
to
Don't let me stand in the way. If you think it's worth
doing go ahead and add my ACK.

Paul Moore

unread,
Jul 14, 2016, 6:10:06 PM7/14/16
to
I think it's a reasonable patch and I'm at that point in the day where
I'm looking for distractions so I just added it to the selinux#next
queue; once the merge window closes I'll rotate this into my next
branch.

--
paul moore
www.paul-moore.com

Javier Martinez Canillas

unread,
Jul 15, 2016, 7:40:06 AM7/15/16
to
Hello Paul,

On 07/14/2016 06:06 PM, Paul Moore wrote:

[snip]

>
> I think it's a reasonable patch and I'm at that point in the day where
> I'm looking for distractions so I just added it to the selinux#next
> queue; once the merge window closes I'll rotate this into my next
> branch.
>

Great, thanks a lot for your help.
0 new messages