diff mbox series

[05/12] s390: zcrypt: initialize variables before_use

Message ID 20190408212648.2407234-5-arnd@arndb.de
State New
Headers show
Series [01/12] s390: remove -fno-strength-reduce flag | expand

Commit Message

Arnd Bergmann April 8, 2019, 9:26 p.m. UTC
The 'func_code' variable gets printed in debug statements without
a prior initialization in multiple functions, as reported when building
with clang:

drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true
      [-Wsometimes-uninitialized]
        if (mex->outputdatalength < mex->inputdatalength) {
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here
        trace_s390_zcrypt_rep(mex, func_code, rc,
                                   ^~~~~~~~~
drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false
        if (mex->outputdatalength < mex->inputdatalength) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning
        unsigned int func_code;
                              ^

Add initializations to all affected code paths to shut up the warning
and make the warning output consistent.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/s390/crypto/zcrypt_api.c | 4 ++++
 1 file changed, 4 insertions(+)

-- 
2.20.0

Comments

Nathan Chancellor April 8, 2019, 10:31 p.m. UTC | #1
On Mon, Apr 08, 2019 at 11:26:18PM +0200, Arnd Bergmann wrote:
> The 'func_code' variable gets printed in debug statements without

> a prior initialization in multiple functions, as reported when building

> with clang:

> 

> drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true

>       [-Wsometimes-uninitialized]

>         if (mex->outputdatalength < mex->inputdatalength) {

>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here

>         trace_s390_zcrypt_rep(mex, func_code, rc,

>                                    ^~~~~~~~~

> drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false

>         if (mex->outputdatalength < mex->inputdatalength) {

>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning

>         unsigned int func_code;

>                               ^

> 

> Add initializations to all affected code paths to shut up the warning

> and make the warning output consistent.

> 

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


I'll never get used to seeing negative numbers assigned to unsigned
integers...

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>


> ---

>  drivers/s390/crypto/zcrypt_api.c | 4 ++++

>  1 file changed, 4 insertions(+)

> 

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c

> index eb93c2d27d0a..23472063d9a8 100644

> --- a/drivers/s390/crypto/zcrypt_api.c

> +++ b/drivers/s390/crypto/zcrypt_api.c

> @@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,

>  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);

>  

>  	if (mex->outputdatalength < mex->inputdatalength) {

> +		func_code = -1;

>  		rc = -EINVAL;

>  		goto out;

>  	}

> @@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,

>  	trace_s390_zcrypt_req(crt, TP_ICARSACRT);

>  

>  	if (crt->outputdatalength < crt->inputdatalength) {

> +		func_code = -1;

>  		rc = -EINVAL;

>  		goto out;

>  	}

> @@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,

>  

>  		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);

>  		if (!targets) {

> +			func_code = -1;

>  			rc = -ENOMEM;

>  			goto out;

>  		}

> @@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,

>  		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;

>  		if (copy_from_user(targets, uptr,

>  				   target_num * sizeof(*targets))) {

> +			func_code = -1;

>  			rc = -EFAULT;

>  			goto out_free;

>  		}

> -- 

> 2.20.0

>
Harald Freudenberger April 9, 2019, 9:54 a.m. UTC | #2
On 08.04.19 23:26, Arnd Bergmann wrote:
> The 'func_code' variable gets printed in debug statements without

> a prior initialization in multiple functions, as reported when building

> with clang:

>

> drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true

>       [-Wsometimes-uninitialized]

>         if (mex->outputdatalength < mex->inputdatalength) {

>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here

>         trace_s390_zcrypt_rep(mex, func_code, rc,

>                                    ^~~~~~~~~

> drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false

>         if (mex->outputdatalength < mex->inputdatalength) {

>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning

>         unsigned int func_code;

>                               ^

>

> Add initializations to all affected code paths to shut up the warning

> and make the warning output consistent.

>

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/s390/crypto/zcrypt_api.c | 4 ++++

>  1 file changed, 4 insertions(+)

>

> diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c

> index eb93c2d27d0a..23472063d9a8 100644

> --- a/drivers/s390/crypto/zcrypt_api.c

> +++ b/drivers/s390/crypto/zcrypt_api.c

> @@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,

>  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);

>  

>  	if (mex->outputdatalength < mex->inputdatalength) {

> +		func_code = -1;

>  		rc = -EINVAL;

>  		goto out;

>  	}

> @@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,

>  	trace_s390_zcrypt_req(crt, TP_ICARSACRT);

>  

>  	if (crt->outputdatalength < crt->inputdatalength) {

> +		func_code = -1;

>  		rc = -EINVAL;

>  		goto out;

>  	}

> @@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,

>  

>  		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);

>  		if (!targets) {

> +			func_code = -1;

>  			rc = -ENOMEM;

>  			goto out;

>  		}

> @@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,

>  		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;

>  		if (copy_from_user(targets, uptr,

>  				   target_num * sizeof(*targets))) {

> +			func_code = -1;

>  			rc = -EFAULT;

>  			goto out_free;

>  		}

Thanks Arnd, but as Nathan already wrote, I'd prefer to have the
variable initialized with 0 instead of -1.
If you agree with this, I'll rewrite the patch and apply it to our
internal git and it will appear at kernel org with the next s390 code merge then.
regards Harald Freudenberger
Arnd Bergmann April 9, 2019, 10:13 a.m. UTC | #3
On Tue, Apr 9, 2019 at 11:54 AM Harald Freudenberger
<freude@linux.ibm.com> wrote:
> On 08.04.19 23:26, Arnd Bergmann wrote:


> Thanks Arnd, but as Nathan already wrote, I'd prefer to have the

> variable initialized with 0 instead of -1.

> If you agree with this, I'll rewrite the patch and apply it to our

> internal git and it will appear at kernel org with the next s390 code merge then.


That's fine with me, I don't care about the specific value that gets
assigned here.

Thanks,

      Arnd
Martin Schwidefsky April 10, 2019, 3:59 p.m. UTC | #4
On Tue, 9 Apr 2019 11:54:30 +0200
Harald Freudenberger <freude@linux.ibm.com> wrote:

> On 08.04.19 23:26, Arnd Bergmann wrote:

> > The 'func_code' variable gets printed in debug statements without

> > a prior initialization in multiple functions, as reported when building

> > with clang:

> >

> > drivers/s390/crypto/zcrypt_api.c:659:6: warning: variable 'func_code' is used uninitialized whenever 'if' condition is true

> >       [-Wsometimes-uninitialized]

> >         if (mex->outputdatalength < mex->inputdatalength) {

> >             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> > drivers/s390/crypto/zcrypt_api.c:725:29: note: uninitialized use occurs here

> >         trace_s390_zcrypt_rep(mex, func_code, rc,

> >                                    ^~~~~~~~~

> > drivers/s390/crypto/zcrypt_api.c:659:2: note: remove the 'if' if its condition is always false

> >         if (mex->outputdatalength < mex->inputdatalength) {

> >         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> > drivers/s390/crypto/zcrypt_api.c:654:24: note: initialize the variable 'func_code' to silence this warning

> >         unsigned int func_code;

> >                               ^

> >

> > Add initializations to all affected code paths to shut up the warning

> > and make the warning output consistent.

> >

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> > ---

> >  drivers/s390/crypto/zcrypt_api.c | 4 ++++

> >  1 file changed, 4 insertions(+)

> >

> > diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c

> > index eb93c2d27d0a..23472063d9a8 100644

> > --- a/drivers/s390/crypto/zcrypt_api.c

> > +++ b/drivers/s390/crypto/zcrypt_api.c

> > @@ -657,6 +657,7 @@ static long zcrypt_rsa_modexpo(struct ap_perms *perms,

> >  	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);

> >  

> >  	if (mex->outputdatalength < mex->inputdatalength) {

> > +		func_code = -1;

> >  		rc = -EINVAL;

> >  		goto out;

> >  	}

> > @@ -739,6 +740,7 @@ static long zcrypt_rsa_crt(struct ap_perms *perms,

> >  	trace_s390_zcrypt_req(crt, TP_ICARSACRT);

> >  

> >  	if (crt->outputdatalength < crt->inputdatalength) {

> > +		func_code = -1;

> >  		rc = -EINVAL;

> >  		goto out;

> >  	}

> > @@ -946,6 +948,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,

> >  

> >  		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);

> >  		if (!targets) {

> > +			func_code = -1;

> >  			rc = -ENOMEM;

> >  			goto out;

> >  		}

> > @@ -953,6 +956,7 @@ static long zcrypt_send_ep11_cprb(struct ap_perms *perms,

> >  		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;

> >  		if (copy_from_user(targets, uptr,

> >  				   target_num * sizeof(*targets))) {

> > +			func_code = -1;

> >  			rc = -EFAULT;

> >  			goto out_free;

> >  		}  

> Thanks Arnd, but as Nathan already wrote, I'd prefer to have the

> variable initialized with 0 instead of -1.

> If you agree with this, I'll rewrite the patch and apply it to our

> internal git and it will appear at kernel org with the next s390 code merge then.


Do we agreement on func_coed=0 for this one ?

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.
Arnd Bergmann April 10, 2019, 6:57 p.m. UTC | #5
On Wed, Apr 10, 2019 at 5:59 PM Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
> On Tue, 9 Apr 2019 11:54:30 +0200 Harald Freudenberger <freude@linux.ibm.com> wrote:

> > On 08.04.19 23:26, Arnd Bergmann wrote:

> > >             }

> > Thanks Arnd, but as Nathan already wrote, I'd prefer to have the

> > variable initialized with 0 instead of -1.

> > If you agree with this, I'll rewrite the patch and apply it to our

> > internal git and it will appear at kernel org with the next s390 code merge then.

>

> Do we agreement on func_coed=0 for this one ?


Yes, I think that was the consensus.

       Arnd
Martin Schwidefsky April 11, 2019, 7:40 a.m. UTC | #6
On Wed, 10 Apr 2019 20:57:21 +0200
Arnd Bergmann <arnd@arndb.de> wrote:

> On Wed, Apr 10, 2019 at 5:59 PM Martin Schwidefsky

> <schwidefsky@de.ibm.com> wrote:

> > On Tue, 9 Apr 2019 11:54:30 +0200 Harald Freudenberger <freude@linux.ibm.com> wrote:  

> > > On 08.04.19 23:26, Arnd Bergmann wrote:  

> > > >             }  

> > > Thanks Arnd, but as Nathan already wrote, I'd prefer to have the

> > > variable initialized with 0 instead of -1.

> > > If you agree with this, I'll rewrite the patch and apply it to our

> > > internal git and it will appear at kernel org with the next s390 code merge then.  

> >

> > Do we agreement on func_coed=0 for this one ?  

> 

> Yes, I think that was the consensus.

> 

>        Arnd

> 


Ok, committed with func_code=0.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.
diff mbox series

Patch

diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c
index eb93c2d27d0a..23472063d9a8 100644
--- a/drivers/s390/crypto/zcrypt_api.c
+++ b/drivers/s390/crypto/zcrypt_api.c
@@ -657,6 +657,7 @@  static long zcrypt_rsa_modexpo(struct ap_perms *perms,
 	trace_s390_zcrypt_req(mex, TP_ICARSAMODEXPO);
 
 	if (mex->outputdatalength < mex->inputdatalength) {
+		func_code = -1;
 		rc = -EINVAL;
 		goto out;
 	}
@@ -739,6 +740,7 @@  static long zcrypt_rsa_crt(struct ap_perms *perms,
 	trace_s390_zcrypt_req(crt, TP_ICARSACRT);
 
 	if (crt->outputdatalength < crt->inputdatalength) {
+		func_code = -1;
 		rc = -EINVAL;
 		goto out;
 	}
@@ -946,6 +948,7 @@  static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
 
 		targets = kcalloc(target_num, sizeof(*targets), GFP_KERNEL);
 		if (!targets) {
+			func_code = -1;
 			rc = -ENOMEM;
 			goto out;
 		}
@@ -953,6 +956,7 @@  static long zcrypt_send_ep11_cprb(struct ap_perms *perms,
 		uptr = (struct ep11_target_dev __force __user *) xcrb->targets;
 		if (copy_from_user(targets, uptr,
 				   target_num * sizeof(*targets))) {
+			func_code = -1;
 			rc = -EFAULT;
 			goto out_free;
 		}