linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* gcc-6.0 warnings for scsi
@ 2016-03-14 14:29 Arnd Bergmann
  2016-03-14 14:29 ` [PATCH 1/3] aacraid: add missing curly braces Arnd Bergmann
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:29 UTC (permalink / raw)
  To: martin.petersen, James.Bottomley; +Cc: linux-scsi, linux-kernel

gcc-6 found a couple of bugs in scsi drivers that are not
yet fixed in linux-next.

In all three cases, the code is indented in a rather misleading
way, but actually correct. I think it would be good to have
this fixed in 4.6, but no backports are needed even though
the problems have all been around for a while.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/3] aacraid: add missing curly braces
  2016-03-14 14:29 gcc-6.0 warnings for scsi Arnd Bergmann
@ 2016-03-14 14:29 ` Arnd Bergmann
  2016-03-18 19:20   ` Martin K. Petersen
                     ` (2 more replies)
  2016-03-14 14:29 ` [PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann
  2016-03-14 14:29 ` [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler Arnd Bergmann
  2 siblings, 3 replies; 16+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:29 UTC (permalink / raw)
  To: martin.petersen, James.Bottomley, Adaptec OEM Raid Solutions,
	James E.J. Bottomley
  Cc: linux-scsi, linux-kernel, Arnd Bergmann, Johannes Thumshirn,
	Tomas Henzl, Mahesh Rajashekhara, Raghava Aditya Renukunta,
	Fengguang Wu

gcc-6 warns about obviously wrong indentation for newly added
code in aac_slave_configure():

drivers/scsi/aacraid/linit.c: In function 'aac_slave_configure':
drivers/scsi/aacraid/linit.c:458:3: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
   sdev->tagged_supported = 1;
   ^~~~
drivers/scsi/aacraid/linit.c:455:4: note: ...this 'else' clause, but it is not

gcc is correct, and evidently this was meant to be within the curly
braces that should have been there to start with. This patch adds
them, which avoids the warning and makes it clear what was intended
here.

Nothing changes in behavior because in the 'if' block, the
sdev->tagged_supported flag is known to be set already.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 6bf3b630d0a7 ("aacraid: SCSI blk tag support")
---
 drivers/scsi/aacraid/linit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 21a67ed047e8..ff6caab8cc8b 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -452,10 +452,11 @@ static int aac_slave_configure(struct scsi_device *sdev)
 		else if (depth < 2)
 			depth = 2;
 		scsi_change_queue_depth(sdev, depth);
-	} else
+	} else {
 		scsi_change_queue_depth(sdev, 1);
 
 		sdev->tagged_supported = 1;
+	}
 
 	return 0;
 }
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] lpfc: fix misleading indentation
  2016-03-14 14:29 gcc-6.0 warnings for scsi Arnd Bergmann
  2016-03-14 14:29 ` [PATCH 1/3] aacraid: add missing curly braces Arnd Bergmann
@ 2016-03-14 14:29 ` Arnd Bergmann
  2016-03-14 15:19   ` Hannes Reinecke
                     ` (2 more replies)
  2016-03-14 14:29 ` [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler Arnd Bergmann
  2 siblings, 3 replies; 16+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:29 UTC (permalink / raw)
  To: martin.petersen, James.Bottomley, James Smart, Dick Kennedy,
	James E.J. Bottomley
  Cc: linux-scsi, linux-kernel, Arnd Bergmann, Hannes Reinecke,
	Sebastian Herbszt

gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array()
call in lpfc_online(), which clearly doesn't look right:

drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online':
drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
   lpfc_destroy_vport_work_array(phba, vports);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not
  if (vports != NULL)
  ^~

Looking at the patch that introduced this code, it's clear that the
behavior is correct and the indentation is wrong.

This fixes the indentation and adds curly braces around the previous
if() block for clarity, as that is most likely what caused the code
to be misindented in the first place.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list")
---
 drivers/scsi/lpfc/lpfc_init.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index a544366a367e..f57d02c3b6cf 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -2860,7 +2860,7 @@ lpfc_online(struct lpfc_hba *phba)
 	}
 
 	vports = lpfc_create_vport_work_array(phba);
-	if (vports != NULL)
+	if (vports != NULL) {
 		for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
 			struct Scsi_Host *shost;
 			shost = lpfc_shost_from_vport(vports[i]);
@@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
 			}
 			spin_unlock_irq(shost->host_lock);
 		}
-		lpfc_destroy_vport_work_array(phba, vports);
+	}
+	lpfc_destroy_vport_work_array(phba, vports);
 
 	lpfc_unblock_mgmt_io(phba);
 	return 0;
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler
  2016-03-14 14:29 gcc-6.0 warnings for scsi Arnd Bergmann
  2016-03-14 14:29 ` [PATCH 1/3] aacraid: add missing curly braces Arnd Bergmann
  2016-03-14 14:29 ` [PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann
@ 2016-03-14 14:29 ` Arnd Bergmann
  2016-03-14 15:20   ` Hannes Reinecke
                     ` (2 more replies)
  2 siblings, 3 replies; 16+ messages in thread
From: Arnd Bergmann @ 2016-03-14 14:29 UTC (permalink / raw)
  To: martin.petersen, James.Bottomley, Kashyap Desai, Sumit Saxena,
	Uday Lingala, James E.J. Bottomley
  Cc: linux-scsi, linux-kernel, Arnd Bergmann, Tomas Henzl,
	Bjorn Helgaas, megaraidlinux.pdl

gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl
function:

drivers/scsi/megaraid/megaraid_sas_base.c: In function 'megasas_mgmt_fw_ioctl':
drivers/scsi/megaraid/megaraid_sas_base.c:6658:4: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
    kbuff_arr[i] = NULL;
    ^~~~~~~~~
drivers/scsi/megaraid/megaraid_sas_base.c:6653:3: note: ...this 'if' clause, but it is not
   if (kbuff_arr[i])
   ^~

The code is actually correct, as there is no downside in clearing a NULL
pointer again.

This clarifies the code and avoids the warning by adding extra curly
braces.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: 90dc9d98f01b ("megaraid_sas : MFI MPT linked list corruption fix")
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 5c08568ccfbf..2627200d4f82 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -6650,12 +6650,13 @@ out:
 	}
 
 	for (i = 0; i < ioc->sge_count; i++) {
-		if (kbuff_arr[i])
+		if (kbuff_arr[i]) {
 			dma_free_coherent(&instance->pdev->dev,
 					  le32_to_cpu(kern_sge32[i].length),
 					  kbuff_arr[i],
 					  le32_to_cpu(kern_sge32[i].phys_addr));
 			kbuff_arr[i] = NULL;
+		}
 	}
 
 	megasas_return_cmd(instance, cmd);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] lpfc: fix misleading indentation
  2016-03-14 14:29 ` [PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann
@ 2016-03-14 15:19   ` Hannes Reinecke
  2016-03-14 15:25     ` Arnd Bergmann
  2016-03-14 22:27   ` Sebastian Herbszt
  2016-03-18 19:22   ` Martin K. Petersen
  2 siblings, 1 reply; 16+ messages in thread
From: Hannes Reinecke @ 2016-03-14 15:19 UTC (permalink / raw)
  To: Arnd Bergmann, martin.petersen, James.Bottomley, James Smart,
	Dick Kennedy, James E.J. Bottomley
  Cc: linux-scsi, linux-kernel, Hannes Reinecke, Sebastian Herbszt

On 03/14/2016 03:29 PM, Arnd Bergmann wrote:
> gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array()
> call in lpfc_online(), which clearly doesn't look right:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online':
> drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
>    lpfc_destroy_vport_work_array(phba, vports);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not
>   if (vports != NULL)
>   ^~
> 
> Looking at the patch that introduced this code, it's clear that the
> behavior is correct and the indentation is wrong.
> 
> This fixes the indentation and adds curly braces around the previous
> if() block for clarity, as that is most likely what caused the code
> to be misindented in the first place.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list")
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index a544366a367e..f57d02c3b6cf 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -2860,7 +2860,7 @@ lpfc_online(struct lpfc_hba *phba)
>  	}
>  
>  	vports = lpfc_create_vport_work_array(phba);
> -	if (vports != NULL)
> +	if (vports != NULL) {
>  		for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
>  			struct Scsi_Host *shost;
>  			shost = lpfc_shost_from_vport(vports[i]);
> @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
>  			}
>  			spin_unlock_irq(shost->host_lock);
>  		}
> -		lpfc_destroy_vport_work_array(phba, vports);
> +	}
> +	lpfc_destroy_vport_work_array(phba, vports);
>  
>  	lpfc_unblock_mgmt_io(phba);
>  	return 0;
> 
Nope.

vports is only valid from within the indentation block, so it should
be moved into it.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler
  2016-03-14 14:29 ` [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler Arnd Bergmann
@ 2016-03-14 15:20   ` Hannes Reinecke
  2016-03-15 11:08   ` Sumit Saxena
  2016-03-18 19:23   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Hannes Reinecke @ 2016-03-14 15:20 UTC (permalink / raw)
  To: Arnd Bergmann, martin.petersen, James.Bottomley, Kashyap Desai,
	Sumit Saxena, Uday Lingala, James E.J. Bottomley
  Cc: linux-scsi, linux-kernel, Tomas Henzl, Bjorn Helgaas, megaraidlinux.pdl

On 03/14/2016 03:29 PM, Arnd Bergmann wrote:
> gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl
> function:
> 
> drivers/scsi/megaraid/megaraid_sas_base.c: In function 'megasas_mgmt_fw_ioctl':
> drivers/scsi/megaraid/megaraid_sas_base.c:6658:4: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
>     kbuff_arr[i] = NULL;
>     ^~~~~~~~~
> drivers/scsi/megaraid/megaraid_sas_base.c:6653:3: note: ...this 'if' clause, but it is not
>    if (kbuff_arr[i])
>    ^~
> 
> The code is actually correct, as there is no downside in clearing a NULL
> pointer again.
> 
> This clarifies the code and avoids the warning by adding extra curly
> braces.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 90dc9d98f01b ("megaraid_sas : MFI MPT linked list corruption fix")
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 5c08568ccfbf..2627200d4f82 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -6650,12 +6650,13 @@ out:
>  	}
>  
>  	for (i = 0; i < ioc->sge_count; i++) {
> -		if (kbuff_arr[i])
> +		if (kbuff_arr[i]) {
>  			dma_free_coherent(&instance->pdev->dev,
>  					  le32_to_cpu(kern_sge32[i].length),
>  					  kbuff_arr[i],
>  					  le32_to_cpu(kern_sge32[i].phys_addr));
>  			kbuff_arr[i] = NULL;
> +		}
>  	}
>  
>  	megasas_return_cmd(instance, cmd);
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] lpfc: fix misleading indentation
  2016-03-14 15:19   ` Hannes Reinecke
@ 2016-03-14 15:25     ` Arnd Bergmann
  2016-03-14 15:26       ` Hannes Reinecke
  0 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-03-14 15:25 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: martin.petersen, James.Bottomley, James Smart, Dick Kennedy,
	James E.J. Bottomley, linux-scsi, linux-kernel, Hannes Reinecke,
	Sebastian Herbszt

On Monday 14 March 2016 16:19:58 Hannes Reinecke wrote:
> >       vports = lpfc_create_vport_work_array(phba);
> > -     if (vports != NULL)
> > +     if (vports != NULL) {
> >               for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
> >                       struct Scsi_Host *shost;
> >                       shost = lpfc_shost_from_vport(vports[i]);
> > @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
> >                       }
> >                       spin_unlock_irq(shost->host_lock);
> >               }
> > -             lpfc_destroy_vport_work_array(phba, vports);
> > +     }
> > +     lpfc_destroy_vport_work_array(phba, vports);
> >  
> >       lpfc_unblock_mgmt_io(phba);
> >       return 0;
> > 
> Nope.
> 
> vports is only valid from within the indentation block, so it should
> be moved into it.
> 
> 

Well, every other user of the function also looks like

	vports = lpfc_create_vport_work_array(phba);
	if (vports != NULL) {
		do_something(vports);
	}
	lpfc_destroy_vport_work_array(phba, vports);

and lpfc_destroy_vport_work_array() does nothing if its argument is NULL.

I still think my patch is the correct fix for the warning.

	Arnd

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] lpfc: fix misleading indentation
  2016-03-14 15:25     ` Arnd Bergmann
@ 2016-03-14 15:26       ` Hannes Reinecke
  2016-03-14 15:48         ` Ewan Milne
  0 siblings, 1 reply; 16+ messages in thread
From: Hannes Reinecke @ 2016-03-14 15:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: martin.petersen, James.Bottomley, James Smart, Dick Kennedy,
	James E.J. Bottomley, linux-scsi, linux-kernel, Hannes Reinecke,
	Sebastian Herbszt

On 03/14/2016 04:25 PM, Arnd Bergmann wrote:
> On Monday 14 March 2016 16:19:58 Hannes Reinecke wrote:
>>>       vports = lpfc_create_vport_work_array(phba);
>>> -     if (vports != NULL)
>>> +     if (vports != NULL) {
>>>               for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
>>>                       struct Scsi_Host *shost;
>>>                       shost = lpfc_shost_from_vport(vports[i]);
>>> @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
>>>                       }
>>>                       spin_unlock_irq(shost->host_lock);
>>>               }
>>> -             lpfc_destroy_vport_work_array(phba, vports);
>>> +     }
>>> +     lpfc_destroy_vport_work_array(phba, vports);
>>>  
>>>       lpfc_unblock_mgmt_io(phba);
>>>       return 0;
>>>
>> Nope.
>>
>> vports is only valid from within the indentation block, so it should
>> be moved into it.
>>
>>
> 
> Well, every other user of the function also looks like
> 
> 	vports = lpfc_create_vport_work_array(phba);
> 	if (vports != NULL) {
> 		do_something(vports);
> 	}
> 	lpfc_destroy_vport_work_array(phba, vports);
> 
> and lpfc_destroy_vport_work_array() does nothing if its argument is NULL.
> 
> I still think my patch is the correct fix for the warning.
> 
Okay, good point.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] lpfc: fix misleading indentation
  2016-03-14 15:26       ` Hannes Reinecke
@ 2016-03-14 15:48         ` Ewan Milne
  0 siblings, 0 replies; 16+ messages in thread
From: Ewan Milne @ 2016-03-14 15:48 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Arnd Bergmann, martin.petersen, James.Bottomley, James Smart,
	Dick Kennedy, James E.J. Bottomley, linux-scsi, linux-kernel,
	Hannes Reinecke, Sebastian Herbszt

On Mon, 2016-03-14 at 16:26 +0100, Hannes Reinecke wrote:
> On 03/14/2016 04:25 PM, Arnd Bergmann wrote:
> > On Monday 14 March 2016 16:19:58 Hannes Reinecke wrote:
> >>>       vports = lpfc_create_vport_work_array(phba);
> >>> -     if (vports != NULL)
> >>> +     if (vports != NULL) {
> >>>               for (i = 0; i <= phba->max_vports && vports[i] != NULL; i++) {
> >>>                       struct Scsi_Host *shost;
> >>>                       shost = lpfc_shost_from_vport(vports[i]);
> >>> @@ -2877,7 +2877,8 @@ lpfc_online(struct lpfc_hba *phba)
> >>>                       }
> >>>                       spin_unlock_irq(shost->host_lock);
> >>>               }
> >>> -             lpfc_destroy_vport_work_array(phba, vports);
> >>> +     }
> >>> +     lpfc_destroy_vport_work_array(phba, vports);
> >>>  
> >>>       lpfc_unblock_mgmt_io(phba);
> >>>       return 0;
> >>>
> >> Nope.
> >>
> >> vports is only valid from within the indentation block, so it should
> >> be moved into it.
> >>
> >>
> > 
> > Well, every other user of the function also looks like
> > 
> > 	vports = lpfc_create_vport_work_array(phba);
> > 	if (vports != NULL) {
> > 		do_something(vports);
> > 	}
> > 	lpfc_destroy_vport_work_array(phba, vports);

Actually the lpfc code is inconsistent about whether the _destroy call
is within the (vports != NULL) test or not, but as you say below it
doesn't matter.

Reviewed-by: Ewan D. Milne <emilne@redhat.com>

> > 
> > and lpfc_destroy_vport_work_array() does nothing if its argument is NULL.
> > 
> > I still think my patch is the correct fix for the warning.
> > 
> Okay, good point.
> 
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> 
> Cheers,
> 
> Hannes

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] lpfc: fix misleading indentation
  2016-03-14 14:29 ` [PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann
  2016-03-14 15:19   ` Hannes Reinecke
@ 2016-03-14 22:27   ` Sebastian Herbszt
  2016-03-18 19:22   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Sebastian Herbszt @ 2016-03-14 22:27 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: martin.petersen, James.Bottomley, James Smart, Dick Kennedy,
	James E.J. Bottomley, linux-scsi, linux-kernel, Hannes Reinecke,
	Sebastian Herbszt

Arnd Bergmann wrote:
> gcc-6 complains about the indentation of the lpfc_destroy_vport_work_array()
> call in lpfc_online(), which clearly doesn't look right:
> 
> drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_online':
> drivers/scsi/lpfc/lpfc_init.c:2880:3: warning: statement is indented as if it were guarded by... [-Wmisleading-indentation]
>    lpfc_destroy_vport_work_array(phba, vports);
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/scsi/lpfc/lpfc_init.c:2863:2: note: ...this 'if' clause, but it is not
>   if (vports != NULL)
>   ^~
> 
> Looking at the patch that introduced this code, it's clear that the
> behavior is correct and the indentation is wrong.
> 
> This fixes the indentation and adds curly braces around the previous
> if() block for clarity, as that is most likely what caused the code
> to be misindented in the first place.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 549e55cd2a1b ("[SCSI] lpfc 8.2.2 : Fix locking around HBA's port_list")
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Sebastian Herbszt <herbszt@gmx.de>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler
  2016-03-14 14:29 ` [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler Arnd Bergmann
  2016-03-14 15:20   ` Hannes Reinecke
@ 2016-03-15 11:08   ` Sumit Saxena
  2016-03-18 19:23   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Sumit Saxena @ 2016-03-15 11:08 UTC (permalink / raw)
  To: Arnd Bergmann, martin.petersen, James.Bottomley, Kashyap Desai,
	Sumit Saxena, Uday Lingala, James E.J. Bottomley
  Cc: linux-scsi, linux-kernel, Tomas Henzl, Bjorn Helgaas, megaraidlinux.pdl

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Monday, March 14, 2016 8:00 PM
> To: martin.petersen@oracle.com; James.Bottomley@HansenPartnership.com;
> Kashyap Desai; Sumit Saxena; Uday Lingala; James E.J. Bottomley
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Arnd
Bergmann;
> Tomas Henzl; Bjorn Helgaas; megaraidlinux.pdl@avagotech.com
> Subject: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl
handler
>
> gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl
> function:
>
> drivers/scsi/megaraid/megaraid_sas_base.c: In function
> 'megasas_mgmt_fw_ioctl':
> drivers/scsi/megaraid/megaraid_sas_base.c:6658:4: warning: statement is
> indented as if it were guarded by... [-Wmisleading-indentation]
>     kbuff_arr[i] = NULL;
>     ^~~~~~~~~
> drivers/scsi/megaraid/megaraid_sas_base.c:6653:3: note: ...this 'if'
clause, but
> it is not
>    if (kbuff_arr[i])
>    ^~
>
> The code is actually correct, as there is no downside in clearing a NULL
pointer
> again.
>
> This clarifies the code and avoids the warning by adding extra curly
braces.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 90dc9d98f01b ("megaraid_sas : MFI MPT linked list corruption
fix")
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 5c08568ccfbf..2627200d4f82 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -6650,12 +6650,13 @@ out:
>  	}
>
>  	for (i = 0; i < ioc->sge_count; i++) {
> -		if (kbuff_arr[i])
> +		if (kbuff_arr[i]) {
>  			dma_free_coherent(&instance->pdev->dev,
>
le32_to_cpu(kern_sge32[i].length),
>  					  kbuff_arr[i],
>
> le32_to_cpu(kern_sge32[i].phys_addr));
>  			kbuff_arr[i] = NULL;
> +		}
>  	}
>
>  	megasas_return_cmd(instance, cmd);

Acked-by: Sumit Saxena <sumit.saxena@broadcom.com>

> --
> 2.7.0

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] aacraid: add missing curly braces
  2016-03-14 14:29 ` [PATCH 1/3] aacraid: add missing curly braces Arnd Bergmann
@ 2016-03-18 19:20   ` Martin K. Petersen
  2016-03-18 20:50   ` Raghava Aditya Renukunta
  2016-03-22  0:26   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2016-03-18 19:20 UTC (permalink / raw)
  To: Raghava Aditya Renukunta
  Cc: martin.petersen, James.Bottomley, Adaptec OEM Raid Solutions,
	linux-scsi, linux-kernel, Johannes Thumshirn, Tomas Henzl,
	Mahesh Rajashekhara, Fengguang Wu, Arnd Bergmann

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

Raghava,

Please review.

Arnd> gcc-6 warns about obviously wrong indentation for newly added code
Arnd> in aac_slave_configure():

https://patchwork.kernel.org/patch/8579681/

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] lpfc: fix misleading indentation
  2016-03-14 14:29 ` [PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann
  2016-03-14 15:19   ` Hannes Reinecke
  2016-03-14 22:27   ` Sebastian Herbszt
@ 2016-03-18 19:22   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2016-03-18 19:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: martin.petersen, James.Bottomley, James Smart, Dick Kennedy,
	James E.J. Bottomley, linux-scsi, linux-kernel, Hannes Reinecke,
	Sebastian Herbszt

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

Arnd> gcc-6 complains about the indentation of the
Arnd> lpfc_destroy_vport_work_array() call in lpfc_online(), which
Arnd> clearly doesn't look right:

Applied to 4.6/scsi-fixes.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler
  2016-03-14 14:29 ` [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler Arnd Bergmann
  2016-03-14 15:20   ` Hannes Reinecke
  2016-03-15 11:08   ` Sumit Saxena
@ 2016-03-18 19:23   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2016-03-18 19:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: martin.petersen, James.Bottomley, Kashyap Desai, Sumit Saxena,
	Uday Lingala, James E.J. Bottomley, linux-scsi, linux-kernel,
	Tomas Henzl, Bjorn Helgaas, megaraidlinux.pdl

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

Arnd> gcc-6 found a dubious indentation in the megasas_mgmt_fw_ioctl
Arnd> function:

Applied to 4.6/scsi-fixes.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/3] aacraid: add missing curly braces
  2016-03-14 14:29 ` [PATCH 1/3] aacraid: add missing curly braces Arnd Bergmann
  2016-03-18 19:20   ` Martin K. Petersen
@ 2016-03-18 20:50   ` Raghava Aditya Renukunta
  2016-03-22  0:26   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Raghava Aditya Renukunta @ 2016-03-18 20:50 UTC (permalink / raw)
  To: Arnd Bergmann, martin.petersen, James.Bottomley,
	Adaptec OEM Raid Solutions, James E.J. Bottomley
  Cc: linux-scsi, linux-kernel, Johannes Thumshirn, Tomas Henzl,
	Mahesh Rajashekhara, Fengguang Wu


> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Monday, March 14, 2016 7:30 AM
> To: martin.petersen@oracle.com;
> James.Bottomley@HansenPartnership.com; Adaptec OEM Raid Solutions;
> James E.J. Bottomley
> Cc: linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; Arnd
> Bergmann; Johannes Thumshirn; Tomas Henzl; Mahesh Rajashekhara;
> Raghava Aditya Renukunta; Fengguang Wu
> Subject: [PATCH 1/3] aacraid: add missing curly braces
> 
> gcc-6 warns about obviously wrong indentation for newly added
> code in aac_slave_configure():
> 
> drivers/scsi/aacraid/linit.c: In function 'aac_slave_configure':
> drivers/scsi/aacraid/linit.c:458:3: warning: statement is indented as if it were
> guarded by... [-Wmisleading-indentation]
>    sdev->tagged_supported = 1;
>    ^~~~
> drivers/scsi/aacraid/linit.c:455:4: note: ...this 'else' clause, but it is not
> 
> gcc is correct, and evidently this was meant to be within the curly
> braces that should have been there to start with. This patch adds
> them, which avoids the warning and makes it clear what was intended
> here.
> 
> Nothing changes in behavior because in the 'if' block, the
> sdev->tagged_supported flag is known to be set already.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 6bf3b630d0a7 ("aacraid: SCSI blk tag support")
> ---
>  drivers/scsi/aacraid/linit.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 21a67ed047e8..ff6caab8cc8b 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -452,10 +452,11 @@ static int aac_slave_configure(struct scsi_device
> *sdev)
>  		else if (depth < 2)
>  			depth = 2;
>  		scsi_change_queue_depth(sdev, depth);
> -	} else
> +	} else {
>  		scsi_change_queue_depth(sdev, 1);
>  		sdev->tagged_supported = 1;
> +	}
> 
>  	return 0;
>  }
> --
> 2.7.0

Reviewed-by: Raghava Aditya Renukunta < raghavaaditya.renukunta@pmcs.com >

Regards,
Raghava Aditya

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/3] aacraid: add missing curly braces
  2016-03-14 14:29 ` [PATCH 1/3] aacraid: add missing curly braces Arnd Bergmann
  2016-03-18 19:20   ` Martin K. Petersen
  2016-03-18 20:50   ` Raghava Aditya Renukunta
@ 2016-03-22  0:26   ` Martin K. Petersen
  2 siblings, 0 replies; 16+ messages in thread
From: Martin K. Petersen @ 2016-03-22  0:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: martin.petersen, James.Bottomley, Adaptec OEM Raid Solutions,
	James E.J. Bottomley, linux-scsi, linux-kernel,
	Johannes Thumshirn, Tomas Henzl, Mahesh Rajashekhara,
	Raghava Aditya Renukunta, Fengguang Wu

>>>>> "Arnd" == Arnd Bergmann <arnd@arndb.de> writes:

Arnd> gcc-6 warns about obviously wrong indentation for newly added code
Arnd> in aac_slave_configure():

Applied to 4.6/scsi-fixes.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-03-22  0:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-14 14:29 gcc-6.0 warnings for scsi Arnd Bergmann
2016-03-14 14:29 ` [PATCH 1/3] aacraid: add missing curly braces Arnd Bergmann
2016-03-18 19:20   ` Martin K. Petersen
2016-03-18 20:50   ` Raghava Aditya Renukunta
2016-03-22  0:26   ` Martin K. Petersen
2016-03-14 14:29 ` [PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann
2016-03-14 15:19   ` Hannes Reinecke
2016-03-14 15:25     ` Arnd Bergmann
2016-03-14 15:26       ` Hannes Reinecke
2016-03-14 15:48         ` Ewan Milne
2016-03-14 22:27   ` Sebastian Herbszt
2016-03-18 19:22   ` Martin K. Petersen
2016-03-14 14:29 ` [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler Arnd Bergmann
2016-03-14 15:20   ` Hannes Reinecke
2016-03-15 11:08   ` Sumit Saxena
2016-03-18 19:23   ` Martin K. Petersen

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).