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

[Bhumika Goyal]

On Wed, Aug 9, 2017 at 11:22 AM, Heiko Carstens
<heiko.carstens@xxxxxxxxxx> 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@xxxxxxxxx>
>> ---
>>  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