gcc-6.0 warnings for scsi

gcc-6.0 warnings for scsi

[Arnd Bergmann]

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.

[PATCH 1/3] aacraid: add missing curly braces

[Arnd Bergmann]

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

Re: [PATCH 1/3] aacraid: add missing curly braces

[Martin K. Petersen]

>>>>> "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

RE: [PATCH 1/3] aacraid: add missing curly braces

[Raghava Aditya Renukunta]

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

Re: [PATCH 1/3] aacraid: add missing curly braces

[Martin K. Petersen]

>>>>> "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

[PATCH 2/3] lpfc: fix misleading indentation

[Arnd Bergmann]

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

Re: [PATCH 2/3] lpfc: fix misleading indentation

[Hannes Reinecke]

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)

Re: [PATCH 2/3] lpfc: fix misleading indentation

[Arnd Bergmann]

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

Re: [PATCH 2/3] lpfc: fix misleading indentation

[Hannes Reinecke]

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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] lpfc: fix misleading indentation

[Hannes Reinecke]

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)

Re: [PATCH 2/3] lpfc: fix misleading indentation

[Ewan Milne]

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

Re: [PATCH 2/3] lpfc: fix misleading indentation

[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>

Re: [PATCH 2/3] lpfc: fix misleading indentation

[Martin K. Petersen]

>>>>> "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

[PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler

[Arnd Bergmann]

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

Re: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler

[Hannes Reinecke]

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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler

[Hannes Reinecke]

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)

RE: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler

[Sumit Saxena]

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

Re: [PATCH 3/3] megaraid_sas: add missing curly braces in ioctl handler

[Martin K. Petersen]

>>>>> "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