Re: [PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED

[Mike Snitzer]

On Fri, Jan 22 2021 at  3:43am -0500,
Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx> 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 <arnd@xxxxxxxx>
> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> ---
> Cc: Dmitry Baryshkov <dbaryshkov@xxxxxxxxx>
> ---
>  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 = &key_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 = &key_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