[PATCH] scsi: fix the build warning

[PATCH] scsi: fix the build warning

[zwu.kernel]

From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>

  Initialize csum variable to fix the build warning.

drivers/scsi/scsi_debug.c: In function ‘dif_verify’:
drivers/scsi/scsi_debug.c:1755:3: warning: ‘csum’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  LD      drivers/built-in.o
  CHK     include/generated/uapi/linux/version.h
make[2]: Nothing to be done for `all'.
make[2]: Nothing to be done for `relocs'.

Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..50d70c3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -1733,7 +1733,7 @@ static int do_device_access(struct scsi_cmnd *scmd,
 
 static u16 dif_compute_csum(const void *buf, int len)
 {
-	u16 csum;
+	u16 csum = -1;
 
 	switch (scsi_debug_guard) {
 	case 1:
-- 
1.7.11.7

Re: [PATCH] scsi: fix the build warning

[Joe Perches]

On Thu, 2013-08-22 at 08:44 +0800, zwu.kernel@gmail.com wrote:
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
> 
>   Initialize csum variable to fix the build warning.

Maybe it'd be better to change the variable
scsi_debug_guard type to bool?

Something like:
---
 drivers/scsi/scsi_debug.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..58375c3 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -97,7 +97,7 @@ static const char * scsi_debug_version_date = "20100324";
 #define DEF_D_SENSE   0
 #define DEF_EVERY_NTH   0
 #define DEF_FAKE_RW	0
-#define DEF_GUARD 0
+#define DEF_GUARD false
 #define DEF_LBPU 0
 #define DEF_LBPWS 0
 #define DEF_LBPWS10 0
@@ -169,7 +169,7 @@ static int scsi_debug_dix = DEF_DIX;
 static int scsi_debug_dsense = DEF_D_SENSE;
 static int scsi_debug_every_nth = DEF_EVERY_NTH;
 static int scsi_debug_fake_rw = DEF_FAKE_RW;
-static int scsi_debug_guard = DEF_GUARD;
+static bool scsi_debug_guard = DEF_GUARD;
 static int scsi_debug_lowest_aligned = DEF_LOWEST_ALIGNED;
 static int scsi_debug_max_luns = DEF_MAX_LUNS;
 static int scsi_debug_max_queue = SCSI_DEBUG_CANQUEUE;
@@ -1735,14 +1735,11 @@ static u16 dif_compute_csum(const void *buf, int len)
 {
 	u16 csum;
 
-	switch (scsi_debug_guard) {
-	case 1:
+	if (scsi_debug_guard)
 		csum = ip_compute_csum(buf, len);
-		break;
-	case 0:
+	else
 		csum = cpu_to_be16(crc_t10dif(buf, len));
-		break;
-	}
+
 	return csum;
 }
 
@@ -2736,7 +2733,7 @@ module_param_named(dix, scsi_debug_dix, int, S_IRUGO);
 module_param_named(dsense, scsi_debug_dsense, int, S_IRUGO | S_IWUSR);
 module_param_named(every_nth, scsi_debug_every_nth, int, S_IRUGO | S_IWUSR);
 module_param_named(fake_rw, scsi_debug_fake_rw, int, S_IRUGO | S_IWUSR);
-module_param_named(guard, scsi_debug_guard, int, S_IRUGO);
+module_param_named(guard, scsi_debug_guard, bool, S_IRUGO);
 module_param_named(lbpu, scsi_debug_lbpu, int, S_IRUGO);
 module_param_named(lbpws, scsi_debug_lbpws, int, S_IRUGO);
 module_param_named(lbpws10, scsi_debug_lbpws10, int, S_IRUGO);
@@ -3312,11 +3309,6 @@ static int __init scsi_debug_init(void)
 		return -EINVAL;
 	}
 
-	if (scsi_debug_guard > 1) {
-		printk(KERN_ERR "scsi_debug_init: guard must be 0 or 1\n");
-		return -EINVAL;
-	}
-
 	if (scsi_debug_ato > 1) {
 		printk(KERN_ERR "scsi_debug_init: ato must be 0 or 1\n");
 		return -EINVAL;
@@ -4028,7 +4020,7 @@ static int sdebug_driver_probe(struct device * dev)
 	       (host_prot & SHOST_DIX_TYPE2_PROTECTION) ? " DIX2" : "",
 	       (host_prot & SHOST_DIX_TYPE3_PROTECTION) ? " DIX3" : "");
 
-	if (scsi_debug_guard == 1)
+	if (scsi_debug_guard)
 		scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_IP);
 	else
 		scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_CRC);

Re: [PATCH] scsi: fix the build warning

[Zhi Yong Wu]

HI,

If you'd like, you should draft one patch for this warning.

On Thu, Aug 22, 2013 at 9:02 AM, Joe Perches <joe@perches.com> wrote:
> On Thu, 2013-08-22 at 08:44 +0800, zwu.kernel@gmail.com wrote:
>> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>>
>>   Initialize csum variable to fix the build warning.
>
> Maybe it'd be better to change the variable
> scsi_debug_guard type to bool?
>
> Something like:
> ---
>  drivers/scsi/scsi_debug.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index cb4fefa..58375c3 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -97,7 +97,7 @@ static const char * scsi_debug_version_date = "20100324";
>  #define DEF_D_SENSE   0
>  #define DEF_EVERY_NTH   0
>  #define DEF_FAKE_RW    0
> -#define DEF_GUARD 0
> +#define DEF_GUARD false
>  #define DEF_LBPU 0
>  #define DEF_LBPWS 0
>  #define DEF_LBPWS10 0
> @@ -169,7 +169,7 @@ static int scsi_debug_dix = DEF_DIX;
>  static int scsi_debug_dsense = DEF_D_SENSE;
>  static int scsi_debug_every_nth = DEF_EVERY_NTH;
>  static int scsi_debug_fake_rw = DEF_FAKE_RW;
> -static int scsi_debug_guard = DEF_GUARD;
> +static bool scsi_debug_guard = DEF_GUARD;
>  static int scsi_debug_lowest_aligned = DEF_LOWEST_ALIGNED;
>  static int scsi_debug_max_luns = DEF_MAX_LUNS;
>  static int scsi_debug_max_queue = SCSI_DEBUG_CANQUEUE;
> @@ -1735,14 +1735,11 @@ static u16 dif_compute_csum(const void *buf, int len)
>  {
>         u16 csum;
>
> -       switch (scsi_debug_guard) {
> -       case 1:
> +       if (scsi_debug_guard)
>                 csum = ip_compute_csum(buf, len);
> -               break;
> -       case 0:
> +       else
>                 csum = cpu_to_be16(crc_t10dif(buf, len));
> -               break;
> -       }
> +
>         return csum;
>  }
>
> @@ -2736,7 +2733,7 @@ module_param_named(dix, scsi_debug_dix, int, S_IRUGO);
>  module_param_named(dsense, scsi_debug_dsense, int, S_IRUGO | S_IWUSR);
>  module_param_named(every_nth, scsi_debug_every_nth, int, S_IRUGO | S_IWUSR);
>  module_param_named(fake_rw, scsi_debug_fake_rw, int, S_IRUGO | S_IWUSR);
> -module_param_named(guard, scsi_debug_guard, int, S_IRUGO);
> +module_param_named(guard, scsi_debug_guard, bool, S_IRUGO);
>  module_param_named(lbpu, scsi_debug_lbpu, int, S_IRUGO);
>  module_param_named(lbpws, scsi_debug_lbpws, int, S_IRUGO);
>  module_param_named(lbpws10, scsi_debug_lbpws10, int, S_IRUGO);
> @@ -3312,11 +3309,6 @@ static int __init scsi_debug_init(void)
>                 return -EINVAL;
>         }
>
> -       if (scsi_debug_guard > 1) {
> -               printk(KERN_ERR "scsi_debug_init: guard must be 0 or 1\n");
> -               return -EINVAL;
> -       }
> -
>         if (scsi_debug_ato > 1) {
>                 printk(KERN_ERR "scsi_debug_init: ato must be 0 or 1\n");
>                 return -EINVAL;
> @@ -4028,7 +4020,7 @@ static int sdebug_driver_probe(struct device * dev)
>                (host_prot & SHOST_DIX_TYPE2_PROTECTION) ? " DIX2" : "",
>                (host_prot & SHOST_DIX_TYPE3_PROTECTION) ? " DIX3" : "");
>
> -       if (scsi_debug_guard == 1)
> +       if (scsi_debug_guard)
>                 scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_IP);
>         else
>                 scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_CRC);
>
>



-- 
Regards,

Zhi Yong Wu

Re: [PATCH] scsi: fix the build warning

[Joe Perches]

On Thu, 2013-08-22 at 09:05 +0800, Zhi Yong Wu wrote:
> HI,

Hi.

> If you'd like, you should draft one patch for this warning.

Not for me.

I don't get this build warning in the first place and
I think the scsi_debug file is quite old and probably
doesn't need to be changed at all.

I think scsi_debug_guard could be set to a negative
value and then the csum will always be -1.

Dunno if that's a good thing.

Anyway, just a suggestion, do what you want with it.

cheers, Joe

Re: [PATCH] scsi: fix the build warning

[Martin K. Petersen]

>>>>> "Joe" == Joe Perches <joe@perches.com> writes:

Joe> I don't get this build warning in the first place and I think the
Joe> scsi_debug file is quite old and probably doesn't need to be
Joe> changed at all.

guard isn't a boolean, it selects the checksum algorithm used.

Also, I believe Akinobu's recent reorganization of this code in question
fixed the warning.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] scsi: fix the build warning

[Joe Perches]

On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
> >>>>> "Joe" == Joe Perches <joe@perches.com> writes:
> 
> Joe> I don't get this build warning in the first place and I think the
> Joe> scsi_debug file is quite old and probably doesn't need to be
> Joe> changed at all.
> 
> guard isn't a boolean, it selects the checksum algorithm used.

OK, maybe this then...
---
 drivers/scsi/scsi_debug.c | 2 +-
 1 file changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..6fc2831 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3312,7 +3312,7 @@ static int __init scsi_debug_init(void)
 		return -EINVAL;
 	}
 
-	if (scsi_debug_guard > 1) {
+	if (scsi_debug_guard < 0 || scsi_debug_guard > 1) {
 		printk(KERN_ERR "scsi_debug_init: guard must be 0 or 1\n");
 		return -EINVAL;
 	}

Re: [PATCH] scsi: fix the build warning

[Zhi Yong Wu]

On Thu, Aug 22, 2013 at 9:55 AM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
>> >>>>> "Joe" == Joe Perches <joe@perches.com> writes:
>>
>> Joe> I don't get this build warning in the first place and I think the
>> Joe> scsi_debug file is quite old and probably doesn't need to be
>> Joe> changed at all.
>>
>> guard isn't a boolean, it selects the checksum algorithm used.
>
> OK, maybe this then...
> ---
>  drivers/scsi/scsi_debug.c | 2 +-
>  1 file changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index cb4fefa..6fc2831 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -3312,7 +3312,7 @@ static int __init scsi_debug_init(void)
>                 return -EINVAL;
>         }
>
> -       if (scsi_debug_guard > 1) {
> +       if (scsi_debug_guard < 0 || scsi_debug_guard > 1) {
I don't think that it can fix that issue.
>                 printk(KERN_ERR "scsi_debug_init: guard must be 0 or 1\n");
>                 return -EINVAL;
>         }
>
>



-- 
Regards,

Zhi Yong Wu

Re: [PATCH] scsi: fix the build warning

[Joe Perches]

On Thu, 2013-08-22 at 09:59 +0800, Zhi Yong Wu wrote:
> On Thu, Aug 22, 2013 at 9:55 AM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
> >> >>>>> "Joe" == Joe Perches <joe@perches.com> writes:
> >>
> >> Joe> I don't get this build warning in the first place and I think the
> >> Joe> scsi_debug file is quite old and probably doesn't need to be
> >> Joe> changed at all.
> >>
> >> guard isn't a boolean, it selects the checksum algorithm used.
> >
> > OK, maybe this then...
> > ---
> >  drivers/scsi/scsi_debug.c | 2 +-
> >  1 file changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index cb4fefa..6fc2831 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> > @@ -3312,7 +3312,7 @@ static int __init scsi_debug_init(void)
> >                 return -EINVAL;
> >         }
> >
> > -       if (scsi_debug_guard > 1) {
> > +       if (scsi_debug_guard < 0 || scsi_debug_guard > 1) {
> I don't think that it can fix that issue.

No, it doesn't fix any compile warning.  Your patch, if
it's needed, would still apply.

This patch suggestion fixes an issue where debug_guard
could be set to a negative value.

I didn't sign it, it's up to Martin or I suppose James
to actually want it done.

cheers, Joe

Re: [PATCH] scsi: fix the build warning

[Akinobu Mita]

2013/8/22 Martin K. Petersen <martin.petersen@oracle.com>:
>>>>>> "Joe" == Joe Perches <joe@perches.com> writes:
>
> Joe> I don't get this build warning in the first place and I think the
> Joe> scsi_debug file is quite old and probably doesn't need to be
> Joe> changed at all.
>
> guard isn't a boolean, it selects the checksum algorithm used.
>
> Also, I believe Akinobu's recent reorganization of this code in question
> fixed the warning.

Unfortunately, this warning isn't fixed in linux-next, either.
Paul Bolle also sent a patch that fixes the same warning in a little
bit different way.

http://marc.info/?l=linux-scsi&m=137404660520109&w=2

Re: [PATCH] scsi: fix the build warning

[James Bottomley]

On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
> 2013/8/22 Martin K. Petersen <martin.petersen@oracle.com>:
> >>>>>> "Joe" == Joe Perches <joe@perches.com> writes:
> >
> > Joe> I don't get this build warning in the first place and I think the
> > Joe> scsi_debug file is quite old and probably doesn't need to be
> > Joe> changed at all.
> >
> > guard isn't a boolean, it selects the checksum algorithm used.
> >
> > Also, I believe Akinobu's recent reorganization of this code in question
> > fixed the warning.
> 
> Unfortunately, this warning isn't fixed in linux-next, either.
> Paul Bolle also sent a patch that fixes the same warning in a little
> bit different way.

Well, it is and it isn't.  Whether you see the warning seems to depend
on how gcc was built.  My take is that an impossible default case just
to keep some versions of gcc quiet is a bit pointless.

James

Re: [PATCH] scsi: fix the build warning

[Akinobu Mita]

2013/8/22 James Bottomley <jbottomley@parallels.com>:
> On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
>> 2013/8/22 Martin K. Petersen <martin.petersen@oracle.com>:
>> >>>>>> "Joe" == Joe Perches <joe@perches.com> writes:
>> >
>> > Joe> I don't get this build warning in the first place and I think the
>> > Joe> scsi_debug file is quite old and probably doesn't need to be
>> > Joe> changed at all.
>> >
>> > guard isn't a boolean, it selects the checksum algorithm used.
>> >
>> > Also, I believe Akinobu's recent reorganization of this code in question
>> > fixed the warning.
>>
>> Unfortunately, this warning isn't fixed in linux-next, either.
>> Paul Bolle also sent a patch that fixes the same warning in a little
>> bit different way.
>
> Well, it is and it isn't.  Whether you see the warning seems to depend
> on how gcc was built.  My take is that an impossible default case just
> to keep some versions of gcc quiet is a bit pointless.

As Joe said in the other reply, scsi_debug_guard could be a negative
value (scsi_debug_guard > 1 is only prohibited).  So this warning
does not seem a false positive.

Re: [PATCH] scsi: fix the build warning

[Paul Bolle]

On Thu, 2013-08-22 at 23:49 +0900, Akinobu Mita wrote:
> 2013/8/22 James Bottomley <jbottomley@parallels.com>:
> > On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
> >> Unfortunately, this warning isn't fixed in linux-next, either.
> >> Paul Bolle also sent a patch that fixes the same warning in a little
> >> bit different way.
> >
> > Well, it is and it isn't.  Whether you see the warning seems to depend
> > on how gcc was built.  My take is that an impossible default case just
> > to keep some versions of gcc quiet is a bit pointless.
> 
> As Joe said in the other reply, scsi_debug_guard could be a negative
> value (scsi_debug_guard > 1 is only prohibited).  So this warning
> does not seem a false positive.

I too think that GCC is correct here. Perhaps the people not seeing this
warning don't have CONFIG_SCSI_DEBUG set.

A week ago Antonia also submitted a patch to silence this warning
( https://lkml.org/lkml/2013/9/13/649 ). That's at least the third time
someone tried to silence it since it got introduced in the v3.11 cycle.

Akinobu, could you please say how you'd like this warning to be
silenced? Or is an actual fix queued somewhere?


Paul Bolle

Re: [PATCH] scsi: fix the build warning

[Akinobu Mita]

2013/9/20 Paul Bolle <pebolle@tiscali.nl>:
> On Thu, 2013-08-22 at 23:49 +0900, Akinobu Mita wrote:
>> 2013/8/22 James Bottomley <jbottomley@parallels.com>:
>> > On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
>> >> Unfortunately, this warning isn't fixed in linux-next, either.
>> >> Paul Bolle also sent a patch that fixes the same warning in a little
>> >> bit different way.
>> >
>> > Well, it is and it isn't.  Whether you see the warning seems to depend
>> > on how gcc was built.  My take is that an impossible default case just
>> > to keep some versions of gcc quiet is a bit pointless.
>>
>> As Joe said in the other reply, scsi_debug_guard could be a negative
>> value (scsi_debug_guard > 1 is only prohibited).  So this warning
>> does not seem a false positive.
>
> I too think that GCC is correct here. Perhaps the people not seeing this
> warning don't have CONFIG_SCSI_DEBUG set.
>
> A week ago Antonia also submitted a patch to silence this warning
> ( https://lkml.org/lkml/2013/9/13/649 ). That's at least the third time
> someone tried to silence it since it got introduced in the v3.11 cycle.
>
> Akinobu, could you please say how you'd like this warning to be
> silenced? Or is an actual fix queued somewhere?

Yesterday, I sent a patch set which includes two fixes for this issue.
I wish this to be merged and I'll do my best.

http://marc.info/?l=linux-scsi&m=137950732530325&w=2
http://marc.info/?l=linux-scsi&m=137950732530326&w=2

The first one prevents scsi_debug_guard from being a negative value by
changing the type of scsi_debug_guard to 'unsigned int'.

The second one is actually titled sparse warning fix, but it also silences
this GCC warning by chaning from switch statement to if/else statements.

Re: [PATCH] scsi: fix the build warning

[Paul Bolle]

On Fri, 2013-09-20 at 23:32 +0900, Akinobu Mita wrote:
> Yesterday, I sent a patch set which includes two fixes for this issue.
> I wish this to be merged and I'll do my best.

I hadn't yet stumbled onto these patches. Thanks!


Paul Bolle