linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [SCSI] scsi_debug: silence GCC warning
@ 2013-07-15  9:18 Paul Bolle
  2013-07-15 18:48 ` [PATCH v2] " Paul Bolle
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2013-07-15  9:18 UTC (permalink / raw)
  To: James E.J. Bottomley; +Cc: linux-scsi, linux-kernel

Building scsi_debug.o triggers a GCC 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]

This is a false positive, but if we make scsi_debug_guard a bool, we
supply GCC with enough information to determine that csum will not be
used uninitialized. It also allows for a minor cleanup.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) Compile tested only.

1) This warning is apparently introduced in v3.11-rc1 by commit beb40ea42b
("[SCSI] scsi_debug: reduce duplication between prot_verify_read and
prot_verify_write")

 drivers/scsi/scsi_debug.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..98e6436 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -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;
@@ -1736,10 +1736,10 @@ static u16 dif_compute_csum(const void *buf, int len)
 	u16 csum;
 
 	switch (scsi_debug_guard) {
-	case 1:
+	case true:
 		csum = ip_compute_csum(buf, len);
 		break;
-	case 0:
+	case false:
 		csum = cpu_to_be16(crc_t10dif(buf, len));
 		break;
 	}
@@ -2736,7 +2736,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 +3312,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 +4023,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 == true)
 		scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_IP);
 	else
 		scsi_host_set_guard(hpnt, SHOST_DIX_GUARD_CRC);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH v2] [SCSI] scsi_debug: silence GCC warning
  2013-07-15  9:18 [PATCH] [SCSI] scsi_debug: silence GCC warning Paul Bolle
@ 2013-07-15 18:48 ` Paul Bolle
  2013-07-16 15:21   ` Akinobu Mita
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2013-07-15 18:48 UTC (permalink / raw)
  To: James E.J. Bottomley; +Cc: linux-scsi, linux-kernel

Building scsi_debug.o triggers a GCC 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]

This is a false positive. But if we transform the switch statement in
dif_compute_csum() to a straightforward if/else statement we supply GCC
with enough information to determine that csum will not be used
uninitialized.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) v2 because I started to worry whether v1 would change an interface
(ie, the way the "guard" module parameter behaves). This patch is much
simpler, and doesn't change any behavior.

1) Still only compile tested.

 drivers/scsi/scsi_debug.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..7565ec5 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -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 == 1)
 		csum = ip_compute_csum(buf, len);
-		break;
-	case 0:
+	else
 		csum = cpu_to_be16(crc_t10dif(buf, len));
-		break;
-	}
+
 	return csum;
 }
 
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] [SCSI] scsi_debug: silence GCC warning
  2013-07-15 18:48 ` [PATCH v2] " Paul Bolle
@ 2013-07-16 15:21   ` Akinobu Mita
  2013-07-16 15:29     ` Paul Bolle
  0 siblings, 1 reply; 6+ messages in thread
From: Akinobu Mita @ 2013-07-16 15:21 UTC (permalink / raw)
  To: Paul Bolle
  Cc: James E.J. Bottomley, linux-scsi, LKML, Martin K. Petersen,
	Douglas Gilbert

2013/7/16 Paul Bolle <pebolle@tiscali.nl>:
> Building scsi_debug.o triggers a GCC 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]
>
> This is a false positive. But if we transform the switch statement in
> dif_compute_csum() to a straightforward if/else statement we supply GCC
> with enough information to determine that csum will not be used
> uninitialized.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> 0) v2 because I started to worry whether v1 would change an interface
> (ie, the way the "guard" module parameter behaves). This patch is much
> simpler, and doesn't change any behavior.
>
> 1) Still only compile tested.

This one looks good to me.  It would be much better if this commit
log had a reference to the commit that introduced this warning as
you described after '---' in v1 patch.

>  drivers/scsi/scsi_debug.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index cb4fefa..7565ec5 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -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 == 1)
>                 csum = ip_compute_csum(buf, len);
> -               break;
> -       case 0:
> +       else
>                 csum = cpu_to_be16(crc_t10dif(buf, len));
> -               break;
> -       }
> +
>         return csum;
>  }
>
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] [SCSI] scsi_debug: silence GCC warning
  2013-07-16 15:21   ` Akinobu Mita
@ 2013-07-16 15:29     ` Paul Bolle
  2013-07-16 15:42       ` Akinobu Mita
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Bolle @ 2013-07-16 15:29 UTC (permalink / raw)
  To: Akinobu Mita
  Cc: James E.J. Bottomley, linux-scsi, LKML, Martin K. Petersen,
	Douglas Gilbert

On Wed, 2013-07-17 at 00:21 +0900, Akinobu Mita wrote:
> This one looks good to me.

Thanks.

> It would be much better if this commit
> log had a reference to the commit that introduced this warning as
> you described after '---' in v1 patch.

Do you mean that I should submit a v3, with a commit explanation that
also has one or two lines that mention commit beb40ea42b ("[SCSI]
scsi_debug: reduce duplication between prot_verify_read and
prot_verify_write")?


Paul Bolle


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v2] [SCSI] scsi_debug: silence GCC warning
  2013-07-16 15:29     ` Paul Bolle
@ 2013-07-16 15:42       ` Akinobu Mita
  2013-07-17  7:36         ` [PATCH v3] " Paul Bolle
  0 siblings, 1 reply; 6+ messages in thread
From: Akinobu Mita @ 2013-07-16 15:42 UTC (permalink / raw)
  To: Paul Bolle
  Cc: James E.J. Bottomley, linux-scsi, LKML, Martin K. Petersen,
	Douglas Gilbert

2013/7/17 Paul Bolle <pebolle@tiscali.nl>:
> On Wed, 2013-07-17 at 00:21 +0900, Akinobu Mita wrote:
>> This one looks good to me.
>
> Thanks.
>
>> It would be much better if this commit
>> log had a reference to the commit that introduced this warning as
>> you described after '---' in v1 patch.
>
> Do you mean that I should submit a v3, with a commit explanation that
> also has one or two lines that mention commit beb40ea42b ("[SCSI]
> scsi_debug: reduce duplication between prot_verify_read and
> prot_verify_write")?

Yes.  I think it will help someone to understand the background of this patch.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v3] [SCSI] scsi_debug: silence GCC warning
  2013-07-16 15:42       ` Akinobu Mita
@ 2013-07-17  7:36         ` Paul Bolle
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Bolle @ 2013-07-17  7:36 UTC (permalink / raw)
  To: James E.J. Bottomley
  Cc: Akinobu Mita, Martin K. Petersen, Douglas Gilbert, linux-scsi,
	linux-kernel

Building scsi_debug.o triggers a GCC 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]

This warning was apparently introduced (in v3.11-rc1) by commit
beb40ea42b ("[SCSI] scsi_debug: reduce duplication between
prot_verify_read and prot_verify_write"). It is a false positive. But if
we transform the switch statement in dif_compute_csum() to a
straightforward if/else statement we supply GCC with enough information
to determine that csum will not be used uninitialized.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) v2: much simpler, and doesn't change any behavior;
   v3: mention commit bebe40ea42b, as Akinobu suggested.

1) Still only compile tested.

 drivers/scsi/scsi_debug.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index cb4fefa..7565ec5 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -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 == 1)
 		csum = ip_compute_csum(buf, len);
-		break;
-	case 0:
+	else
 		csum = cpu_to_be16(crc_t10dif(buf, len));
-		break;
-	}
+
 	return csum;
 }
 
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-17  7:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-15  9:18 [PATCH] [SCSI] scsi_debug: silence GCC warning Paul Bolle
2013-07-15 18:48 ` [PATCH v2] " Paul Bolle
2013-07-16 15:21   ` Akinobu Mita
2013-07-16 15:29     ` Paul Bolle
2013-07-16 15:42       ` Akinobu Mita
2013-07-17  7:36         ` [PATCH v3] " Paul Bolle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).