Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1704750 > unrolled thread

[PATCH] s390/crypto: make cca_public_sec and cca_token_hdr const

Started byBhumika Goyal <bhumirks@gmail.com>
First post2017-08-06 08:00 +0200
Last post2017-08-09 09:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] s390/crypto: make cca_public_sec and cca_token_hdr const Bhumika Goyal <bhumirks@gmail.com> - 2017-08-06 08:00 +0200
    Re: [PATCH] s390/crypto: make cca_public_sec and cca_token_hdr const Heiko Carstens <heiko.carstens@de.ibm.com> - 2017-08-09 08:00 +0200
      Re: [PATCH] s390/crypto: make cca_public_sec and cca_token_hdr const Bhumika Goyal <bhumirks@gmail.com> - 2017-08-09 09:50 +0200

#1704750 — [PATCH] s390/crypto: make cca_public_sec and cca_token_hdr const

FromBhumika Goyal <bhumirks@gmail.com>
Date2017-08-06 08:00 +0200
Subject[PATCH] s390/crypto: make cca_public_sec and cca_token_hdr const
Message-ID<ubmLv-6Is-3@gated-at.bofh.it>
Declare cca_public_sec and cca_token_hdr structures as const as they are
only used during copy operations.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/s390/crypto/zcrypt_cca_key.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/s390/crypto/zcrypt_cca_key.h b/drivers/s390/crypto/zcrypt_cca_key.h
index 12cff62..7b78ba6 100644
--- a/drivers/s390/crypto/zcrypt_cca_key.h
+++ b/drivers/s390/crypto/zcrypt_cca_key.h
@@ -116,10 +116,10 @@ struct cca_pvt_ext_CRT_sec {
  */
 static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
 {
-	static struct cca_token_hdr static_pub_hdr = {
+	static const struct cca_token_hdr static_pub_hdr = {
 		.token_identifier	=  0x1E,
 	};
-	static struct cca_public_sec static_pub_sec = {
+	static const struct cca_public_sec static_pub_sec = {
 		.section_identifier	=  0x04,
 	};
 	struct {
@@ -176,7 +176,7 @@ static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
  */
 static inline int zcrypt_type6_crt_key(struct ica_rsa_modexpo_crt *crt, void *p)
 {
-	static struct cca_public_sec static_cca_pub_sec = {
+	static const struct cca_public_sec static_cca_pub_sec = {
 		.section_identifier = 4,
 		.section_length = 0x000f,
 		.exponent_len = 0x0003,
-- 
1.9.1

[toc] | [next] | [standalone]


#1707077

FromHeiko Carstens <heiko.carstens@de.ibm.com>
Date2017-08-09 08:00 +0200
Message-ID<ucsca-2Wo-7@gated-at.bofh.it>
In reply to#1704750
On Sun, Aug 06, 2017 at 11:22:27AM +0530, Bhumika Goyal wrote:
> Declare cca_public_sec and cca_token_hdr structures as const as they are
> only used during copy operations.
> 
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
>  drivers/s390/crypto/zcrypt_cca_key.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/s390/crypto/zcrypt_cca_key.h b/drivers/s390/crypto/zcrypt_cca_key.h
> index 12cff62..7b78ba6 100644
> --- a/drivers/s390/crypto/zcrypt_cca_key.h
> +++ b/drivers/s390/crypto/zcrypt_cca_key.h
> @@ -116,10 +116,10 @@ struct cca_pvt_ext_CRT_sec {
>   */
>  static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
>  {
> -	static struct cca_token_hdr static_pub_hdr = {
> +	static const struct cca_token_hdr static_pub_hdr = {
>  		.token_identifier	=  0x1E,
>  	};
> -	static struct cca_public_sec static_pub_sec = {
> +	static const struct cca_public_sec static_pub_sec = {
>  		.section_identifier	=  0x04,
>  	};
>  	struct {
> @@ -176,7 +176,7 @@ static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
>   */
>  static inline int zcrypt_type6_crt_key(struct ica_rsa_modexpo_crt *crt, void *p)
>  {
> -	static struct cca_public_sec static_cca_pub_sec = {
> +	static const struct cca_public_sec static_cca_pub_sec = {
>  		.section_identifier = 4,
>  		.section_length = 0x000f,
>  		.exponent_len = 0x0003,

If you look at the code, it seems to be questionable why these structures
are declared static and not simply on the stack. In the first case we are
talking of an assignment of two single bytes two a structure that has
already been zeroed.
Besides this, the two functions in the header file are very large and
shouldn't be unconditionally inlined anyway. This needs to be cleaned
up. So I'm not at all in favor of a patch that just adds a const keyword in
addition.

[toc] | [prev] | [next] | [standalone]


#1707104

FromBhumika Goyal <bhumirks@gmail.com>
Date2017-08-09 09:50 +0200
Message-ID<uctUB-410-1@gated-at.bofh.it>
In reply to#1707077
On Wed, Aug 9, 2017 at 11:22 AM, Heiko Carstens
<heiko.carstens@de.ibm.com> wrote:
> On Sun, Aug 06, 2017 at 11:22:27AM +0530, Bhumika Goyal wrote:
>> Declare cca_public_sec and cca_token_hdr structures as const as they are
>> only used during copy operations.
>>
>> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
>> ---
>>  drivers/s390/crypto/zcrypt_cca_key.h | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/s390/crypto/zcrypt_cca_key.h b/drivers/s390/crypto/zcrypt_cca_key.h
>> index 12cff62..7b78ba6 100644
>> --- a/drivers/s390/crypto/zcrypt_cca_key.h
>> +++ b/drivers/s390/crypto/zcrypt_cca_key.h
>> @@ -116,10 +116,10 @@ struct cca_pvt_ext_CRT_sec {
>>   */
>>  static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
>>  {
>> -     static struct cca_token_hdr static_pub_hdr = {
>> +     static const struct cca_token_hdr static_pub_hdr = {
>>               .token_identifier       =  0x1E,
>>       };
>> -     static struct cca_public_sec static_pub_sec = {
>> +     static const struct cca_public_sec static_pub_sec = {
>>               .section_identifier     =  0x04,
>>       };
>>       struct {
>> @@ -176,7 +176,7 @@ static inline int zcrypt_type6_mex_key_en(struct ica_rsa_modexpo *mex, void *p)
>>   */
>>  static inline int zcrypt_type6_crt_key(struct ica_rsa_modexpo_crt *crt, void *p)
>>  {
>> -     static struct cca_public_sec static_cca_pub_sec = {
>> +     static const struct cca_public_sec static_cca_pub_sec = {
>>               .section_identifier = 4,
>>               .section_length = 0x000f,
>>               .exponent_len = 0x0003,
>
> If you look at the code, it seems to be questionable why these structures
> are declared static and not simply on the stack. In the first case we are
> talking of an assignment of two single bytes two a structure that has
> already been zeroed.
> Besides this, the two functions in the header file are very large and
> shouldn't be unconditionally inlined anyway. This needs to be cleaned
> up. So I'm not at all in favor of a patch that just adds a const keyword in
> addition.
>

Thanks for the explanation. I think that it would be better to drop the patch.

Thanks,
Bhumika

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web