Re: [PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED
пт, 22 янв. 2021 г. в 11:43, Ahmad Fatoum : > > IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in > or a module, so use it instead of #if defined checking for each > separately. > > The other #if was to avoid a static function defined, but unused > warning. As we now always build the callsite when the function > is defined, we can remove that first #if guard. > > Suggested-by: Arnd Bergmann > Signed-off-by: Ahmad Fatoum Acked-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED
Hello Mike,
On 02.02.21 19:10, Mike Snitzer wrote:
> On Fri, Jan 22 2021 at 3:43am -0500,
> Ahmad Fatoum wrote:
>
>> IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in
>> or a module, so use it instead of #if defined checking for each
>> separately.
>>
>> The other #if was to avoid a static function defined, but unused
>> warning. As we now always build the callsite when the function
>> is defined, we can remove that first #if guard.
>>
>> Suggested-by: Arnd Bergmann
>> Signed-off-by: Ahmad Fatoum
>> ---
>> Cc: Dmitry Baryshkov
>> ---
>> drivers/md/dm-crypt.c | 7 ++-
>> 1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>> index 8c874710f0bc..7eeb9248eda5 100644
>> --- a/drivers/md/dm-crypt.c
>> +++ b/drivers/md/dm-crypt.c
>> @@ -2436,7 +2436,6 @@ static int set_key_user(struct crypt_config *cc,
>> struct key *key)
>> return 0;
>> }
>>
>> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
>> static int set_key_encrypted(struct crypt_config *cc, struct key *key)
>> {
>> const struct encrypted_key_payload *ekp;
>> @@ -2452,7 +2451,6 @@ static int set_key_encrypted(struct crypt_config *cc,
>> struct key *key)
>>
>> return 0;
>> }
>> -#endif /* CONFIG_ENCRYPTED_KEYS */
>>
>> static int crypt_set_keyring_key(struct crypt_config *cc, const char
>> *key_string)
>> {
>> @@ -2482,11 +2480,10 @@ static int crypt_set_keyring_key(struct crypt_config
>> *cc, const char *key_string
>> } else if (!strncmp(key_string, "user:", key_desc - key_string + 1)) {
>> type = _type_user;
>> set_key = set_key_user;
>> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
>> -} else if (!strncmp(key_string, "encrypted:", key_desc - key_string +
>> 1)) {
>> +} else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) &&
>> + !strncmp(key_string, "encrypted:", key_desc - key_string +
>> 1)) {
>> type = _type_encrypted;
>> set_key = set_key_encrypted;
>> -#endif
>> } else {
>> return -EINVAL;
>> }
>> --
>> 2.30.0
>>
>
> I could be mistaken but the point of the previous way used to enable
> this code was to not compile the code at all. How you have it, the
> branch just isn't taken but the compiled code is left to bloat dm-crypt.
>
> Why not leave this as is and follow same pattern in your next patch?
It's safe to assume the compiler will constant-fold the resulting if (0) away.
The kernel coding style documentation got a section on that:
https://www.kernel.org/doc/html/v5.11-rc6/process/coding-style.html#conditional-compilation
Cheers,
Ahmad
>
> Mike
>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED
On Fri, Jan 22 2021 at 3:43am -0500,
Ahmad Fatoum wrote:
> IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in
> or a module, so use it instead of #if defined checking for each
> separately.
>
> The other #if was to avoid a static function defined, but unused
> warning. As we now always build the callsite when the function
> is defined, we can remove that first #if guard.
>
> Suggested-by: Arnd Bergmann
> Signed-off-by: Ahmad Fatoum
> ---
> Cc: Dmitry Baryshkov
> ---
> drivers/md/dm-crypt.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 8c874710f0bc..7eeb9248eda5 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -2436,7 +2436,6 @@ static int set_key_user(struct crypt_config *cc, struct
> key *key)
> return 0;
> }
>
> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
> static int set_key_encrypted(struct crypt_config *cc, struct key *key)
> {
> const struct encrypted_key_payload *ekp;
> @@ -2452,7 +2451,6 @@ static int set_key_encrypted(struct crypt_config *cc,
> struct key *key)
>
> return 0;
> }
> -#endif /* CONFIG_ENCRYPTED_KEYS */
>
> static int crypt_set_keyring_key(struct crypt_config *cc, const char
> *key_string)
> {
> @@ -2482,11 +2480,10 @@ static int crypt_set_keyring_key(struct crypt_config
> *cc, const char *key_string
> } else if (!strncmp(key_string, "user:", key_desc - key_string + 1)) {
> type = _type_user;
> set_key = set_key_user;
> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
> - } else if (!strncmp(key_string, "encrypted:", key_desc - key_string +
> 1)) {
> + } else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) &&
> +!strncmp(key_string, "encrypted:", key_desc - key_string +
> 1)) {
> type = _type_encrypted;
> set_key = set_key_encrypted;
> -#endif
> } else {
> return -EINVAL;
> }
> --
> 2.30.0
>
I could be mistaken but the point of the previous way used to enable
this code was to not compile the code at all. How you have it, the
branch just isn't taken but the compiled code is left to bloat dm-crypt.
Why not leave this as is and follow same pattern in your next patch?
Mike
[PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED
IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in
or a module, so use it instead of #if defined checking for each
separately.
The other #if was to avoid a static function defined, but unused
warning. As we now always build the callsite when the function
is defined, we can remove that first #if guard.
Suggested-by: Arnd Bergmann
Signed-off-by: Ahmad Fatoum
---
Cc: Dmitry Baryshkov
---
drivers/md/dm-crypt.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 8c874710f0bc..7eeb9248eda5 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2436,7 +2436,6 @@ static int set_key_user(struct crypt_config *cc, struct
key *key)
return 0;
}
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
static int set_key_encrypted(struct crypt_config *cc, struct key *key)
{
const struct encrypted_key_payload *ekp;
@@ -2452,7 +2451,6 @@ static int set_key_encrypted(struct crypt_config *cc,
struct key *key)
return 0;
}
-#endif /* CONFIG_ENCRYPTED_KEYS */
static int crypt_set_keyring_key(struct crypt_config *cc, const char
*key_string)
{
@@ -2482,11 +2480,10 @@ static int crypt_set_keyring_key(struct crypt_config
*cc, const char *key_string
} else if (!strncmp(key_string, "user:", key_desc - key_string + 1)) {
type = _type_user;
set_key = set_key_user;
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
- } else if (!strncmp(key_string, "encrypted:", key_desc - key_string +
1)) {
+ } else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) &&
+ !strncmp(key_string, "encrypted:", key_desc - key_string +
1)) {
type = _type_encrypted;
set_key = set_key_encrypted;
-#endif
} else {
return -EINVAL;
}
--
2.30.0
