diff mbox series

[-next] evm: Use IS_ENABLED to initialize .enabled

Message ID 20220606101042.89638-1-xiujianfeng@huawei.com (mailing list archive)
State New
Headers show
Series [-next] evm: Use IS_ENABLED to initialize .enabled | expand

Commit Message

xiujianfeng June 6, 2022, 10:10 a.m. UTC
Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
initialize .enabled, minor simplicity improvement.

Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
---
 security/integrity/evm/evm_main.c | 52 ++++++++++++++-----------------
 1 file changed, 23 insertions(+), 29 deletions(-)

Comments

Ahmad Fatoum June 7, 2022, 6:06 a.m. UTC | #1
On 06.06.22 12:10, Xiu Jianfeng wrote:
> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> initialize .enabled, minor simplicity improvement.
> 
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  security/integrity/evm/evm_main.c | 52 ++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 29 deletions(-)
> 
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index cc88f02c7562..397fea5b3fa6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -36,42 +36,36 @@ static const char * const integrity_status_msg[] = {
>  int evm_hmac_attrs;
>  
>  static struct xattr_list evm_config_default_xattrnames[] = {
> -	{.name = XATTR_NAME_SELINUX,
> -#ifdef CONFIG_SECURITY_SELINUX
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SELINUX,
> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SELINUX)
>  	},
> -	{.name = XATTR_NAME_SMACK,
> -#ifdef CONFIG_SECURITY_SMACK
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACK,
> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SMACK)
>  	},
> -	{.name = XATTR_NAME_SMACKEXEC,
> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACKEXEC,
> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>  	},
> -	{.name = XATTR_NAME_SMACKTRANSMUTE,
> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACKTRANSMUTE,
> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>  	},
> -	{.name = XATTR_NAME_SMACKMMAP,
> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_SMACKMMAP,
> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>  	},
> -	{.name = XATTR_NAME_APPARMOR,
> -#ifdef CONFIG_SECURITY_APPARMOR
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_APPARMOR,
> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_APPARMOR)
>  	},
> -	{.name = XATTR_NAME_IMA,
> -#ifdef CONFIG_IMA_APPRAISE
> -	 .enabled = true
> -#endif
> +	{
> +	 .name = XATTR_NAME_IMA,
> +	 .enabled = IS_ENABLED(CONFIG_IMA_APPRAISE)
>  	},
> -	{.name = XATTR_NAME_CAPS,
> +	{
> +	 .name = XATTR_NAME_CAPS,
>  	 .enabled = true
>  	},
>  };
xiujianfeng June 21, 2022, 10:58 a.m. UTC | #2
Hi, Ahmad

在 2022/6/7 14:06, Ahmad Fatoum 写道:
> On 06.06.22 12:10, Xiu Jianfeng wrote:
>> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
>> initialize .enabled, minor simplicity improvement.
>>
>> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
thank you for the review, and I'm not sure if this patch has been 
picked, so frendly ping here...
>> ---
>>   security/integrity/evm/evm_main.c | 52 ++++++++++++++-----------------
>>   1 file changed, 23 insertions(+), 29 deletions(-)
>>
>> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>> index cc88f02c7562..397fea5b3fa6 100644
>> --- a/security/integrity/evm/evm_main.c
>> +++ b/security/integrity/evm/evm_main.c
>> @@ -36,42 +36,36 @@ static const char * const integrity_status_msg[] = {
>>   int evm_hmac_attrs;
>>   
>>   static struct xattr_list evm_config_default_xattrnames[] = {
>> -	{.name = XATTR_NAME_SELINUX,
>> -#ifdef CONFIG_SECURITY_SELINUX
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SELINUX,
>> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SELINUX)
>>   	},
>> -	{.name = XATTR_NAME_SMACK,
>> -#ifdef CONFIG_SECURITY_SMACK
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACK,
>> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_SMACK)
>>   	},
>> -	{.name = XATTR_NAME_SMACKEXEC,
>> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACKEXEC,
>> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>>   	},
>> -	{.name = XATTR_NAME_SMACKTRANSMUTE,
>> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACKTRANSMUTE,
>> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>>   	},
>> -	{.name = XATTR_NAME_SMACKMMAP,
>> -#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_SMACKMMAP,
>> +	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
>>   	},
>> -	{.name = XATTR_NAME_APPARMOR,
>> -#ifdef CONFIG_SECURITY_APPARMOR
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_APPARMOR,
>> +	 .enabled = IS_ENABLED(CONFIG_SECURITY_APPARMOR)
>>   	},
>> -	{.name = XATTR_NAME_IMA,
>> -#ifdef CONFIG_IMA_APPRAISE
>> -	 .enabled = true
>> -#endif
>> +	{
>> +	 .name = XATTR_NAME_IMA,
>> +	 .enabled = IS_ENABLED(CONFIG_IMA_APPRAISE)
>>   	},
>> -	{.name = XATTR_NAME_CAPS,
>> +	{
>> +	 .name = XATTR_NAME_CAPS,
>>   	 .enabled = true
>>   	},
>>   };
>
Mimi Zohar June 21, 2022, 2:03 p.m. UTC | #3
On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
> Hi, Ahmad
> 
> 在 2022/6/7 14:06, Ahmad Fatoum 写道:
> > On 06.06.22 12:10, Xiu Jianfeng wrote:
> >> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> >> initialize .enabled, minor simplicity improvement.

The difference between using ifdef's and IS_ENABLED is when the
decision is made - build time, run time.   Please update the patch
description providing an explanation for needing to make the decision
at run time.

thanks,

Mimi
Eric Biggers June 22, 2022, 2:17 a.m. UTC | #4
On Tue, Jun 21, 2022 at 10:03:39AM -0400, Mimi Zohar wrote:
> On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
> > Hi, Ahmad
> > 
> > 在 2022/6/7 14:06, Ahmad Fatoum 写道:
> > > On 06.06.22 12:10, Xiu Jianfeng wrote:
> > >> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> > >> initialize .enabled, minor simplicity improvement.
> 
> The difference between using ifdef's and IS_ENABLED is when the
> decision is made - build time, run time.   Please update the patch
> description providing an explanation for needing to make the decision
> at run time.
> 
> thanks,

IS_ENABLED() is a compile time constant.  So the patch looks fine to me.

- Eric
Mimi Zohar June 26, 2022, 4:13 p.m. UTC | #5
On Tue, 2022-06-21 at 19:17 -0700, Eric Biggers wrote:
> On Tue, Jun 21, 2022 at 10:03:39AM -0400, Mimi Zohar wrote:
> > On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
> > > Hi, Ahmad
> > > 
> > > 在 2022/6/7 14:06, Ahmad Fatoum 写道:
> > > > On 06.06.22 12:10, Xiu Jianfeng wrote:
> > > >> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> > > >> initialize .enabled, minor simplicity improvement.
> > 
> > The difference between using ifdef's and IS_ENABLED is when the
> > decision is made - build time, run time.   Please update the patch
> > description providing an explanation for needing to make the decision
> > at run time.
> > 
> > thanks,
> 
> IS_ENABLED() is a compile time constant.  So the patch looks fine to me.

Thanks, Eric, for the clarification.

As LSMs are only builtin, why the need for using IS_ENABLED as opposed
to IS_BUILTIN?

#define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))

thanks,

Mimi
xiujianfeng June 29, 2022, 3:59 a.m. UTC | #6
在 2022/6/27 0:13, Mimi Zohar 写道:
> On Tue, 2022-06-21 at 19:17 -0700, Eric Biggers wrote:
>> On Tue, Jun 21, 2022 at 10:03:39AM -0400, Mimi Zohar wrote:
>>> On Tue, 2022-06-21 at 18:58 +0800, xiujianfeng wrote:
>>>> Hi, Ahmad
>>>>
>>>> 在 2022/6/7 14:06, Ahmad Fatoum 写道:
>>>>> On 06.06.22 12:10, Xiu Jianfeng wrote:
>>>>>> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
>>>>>> initialize .enabled, minor simplicity improvement.
>>> The difference between using ifdef's and IS_ENABLED is when the
>>> decision is made - build time, run time.   Please update the patch
>>> description providing an explanation for needing to make the decision
>>> at run time.
>>>
>>> thanks,
>> IS_ENABLED() is a compile time constant.  So the patch looks fine to me.
> Thanks, Eric, for the clarification.
>
> As LSMs are only builtin, why the need for using IS_ENABLED as opposed
> to IS_BUILTIN?
>
> #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>
> thanks,

I think IS_ENALBED() is a bit more generic, maybe. here is another 
example in rcutorture.c

which uses IS_ENABLED() to initialize the member in structure:

static struct rcu_torture_ops rcu_ops = {

...

.can_boost = IS_ENABLED(CONFIG_RCU_BOOST),

...

};

Do you want me to change IS_ENABLED() to IS_BUILTIN()?

>
> Mimi
>
> .
Mimi Zohar July 7, 2022, 11:15 p.m. UTC | #7
On Mon, 2022-06-06 at 18:10 +0800, Xiu Jianfeng wrote:
> Use IS_ENABLED(CONFIG_XXX) instead of #ifdef/#endif statements to
> initialize .enabled, minor simplicity improvement.
> 
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>

Thanks, Xiu.   This patch is now queued in next-testing.

Mimi
diff mbox series

Patch

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index cc88f02c7562..397fea5b3fa6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -36,42 +36,36 @@  static const char * const integrity_status_msg[] = {
 int evm_hmac_attrs;
 
 static struct xattr_list evm_config_default_xattrnames[] = {
-	{.name = XATTR_NAME_SELINUX,
-#ifdef CONFIG_SECURITY_SELINUX
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SELINUX,
+	 .enabled = IS_ENABLED(CONFIG_SECURITY_SELINUX)
 	},
-	{.name = XATTR_NAME_SMACK,
-#ifdef CONFIG_SECURITY_SMACK
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACK,
+	 .enabled = IS_ENABLED(CONFIG_SECURITY_SMACK)
 	},
-	{.name = XATTR_NAME_SMACKEXEC,
-#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACKEXEC,
+	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
 	},
-	{.name = XATTR_NAME_SMACKTRANSMUTE,
-#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACKTRANSMUTE,
+	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
 	},
-	{.name = XATTR_NAME_SMACKMMAP,
-#ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_SMACKMMAP,
+	 .enabled = IS_ENABLED(CONFIG_EVM_EXTRA_SMACK_XATTRS)
 	},
-	{.name = XATTR_NAME_APPARMOR,
-#ifdef CONFIG_SECURITY_APPARMOR
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_APPARMOR,
+	 .enabled = IS_ENABLED(CONFIG_SECURITY_APPARMOR)
 	},
-	{.name = XATTR_NAME_IMA,
-#ifdef CONFIG_IMA_APPRAISE
-	 .enabled = true
-#endif
+	{
+	 .name = XATTR_NAME_IMA,
+	 .enabled = IS_ENABLED(CONFIG_IMA_APPRAISE)
 	},
-	{.name = XATTR_NAME_CAPS,
+	{
+	 .name = XATTR_NAME_CAPS,
 	 .enabled = true
 	},
 };