From: Zhi Yong Wu <[email protected]>
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 <[email protected]>
---
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
On Thu, 2013-08-22 at 08:44 +0800, [email protected] wrote:
> From: Zhi Yong Wu <[email protected]>
>
> 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);
HI,
If you'd like, you should draft one patch for this warning.
On Thu, Aug 22, 2013 at 9:02 AM, Joe Perches <[email protected]> wrote:
> On Thu, 2013-08-22 at 08:44 +0800, [email protected] wrote:
>> From: Zhi Yong Wu <[email protected]>
>>
>> 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
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
>>>>> "Joe" == Joe Perches <[email protected]> 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
On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
> >>>>> "Joe" == Joe Perches <[email protected]> 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;
}
On Thu, Aug 22, 2013 at 9:55 AM, Joe Perches <[email protected]> wrote:
> On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
>> >>>>> "Joe" == Joe Perches <[email protected]> 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
On Thu, 2013-08-22 at 09:59 +0800, Zhi Yong Wu wrote:
> On Thu, Aug 22, 2013 at 9:55 AM, Joe Perches <[email protected]> wrote:
> > On Wed, 2013-08-21 at 21:25 -0400, Martin K. Petersen wrote:
> >> >>>>> "Joe" == Joe Perches <[email protected]> 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
2013/8/22 Martin K. Petersen <[email protected]>:
>>>>>> "Joe" == Joe Perches <[email protected]> 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
On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
> 2013/8/22 Martin K. Petersen <[email protected]>:
> >>>>>> "Joe" == Joe Perches <[email protected]> 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
2013/8/22 James Bottomley <[email protected]>:
> On Thu, 2013-08-22 at 21:42 +0900, Akinobu Mita wrote:
>> 2013/8/22 Martin K. Petersen <[email protected]>:
>> >>>>>> "Joe" == Joe Perches <[email protected]> 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.
On Thu, 2013-08-22 at 23:49 +0900, Akinobu Mita wrote:
> 2013/8/22 James Bottomley <[email protected]>:
> > 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
2013/9/20 Paul Bolle <[email protected]>:
> On Thu, 2013-08-22 at 23:49 +0900, Akinobu Mita wrote:
>> 2013/8/22 James Bottomley <[email protected]>:
>> > 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.
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