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

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

[Javier Martinez Canillas]

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 <javier@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

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

[Casey Schaufler]

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?

>
> Signed-off-by: Javier Martinez Canillas <javier@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,

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

[Javier Martinez Canillas]

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

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

[Casey Schaufler]

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,

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

[Paul Moore]

On Thu, Jul 14, 2016 at 12:30 PM, Casey Schaufler
<casey@schaufler-ca.com> wrote:
> 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.

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

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

[Casey Schaufler]

On 7/14/2016 12:57 PM, Paul Moore wrote:
> On Thu, Jul 14, 2016 at 12:30 PM, Casey Schaufler
> <casey@schaufler-ca.com> wrote:
>> 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.
> 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.

Don't let me stand in the way. If you think it's worth
doing go ahead and add my ACK.

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

[Paul Moore]

On Thu, Jul 14, 2016 at 4:54 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/14/2016 12:57 PM, Paul Moore wrote:
>> On Thu, Jul 14, 2016 at 12:30 PM, Casey Schaufler
>> <casey@schaufler-ca.com> wrote:
>>> 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.
>>
>> 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.
>
> Don't let me stand in the way. If you think it's worth
> doing go ahead and add my ACK.

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

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

[Javier Martinez Canillas]

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.

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