Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1357301 > unrolled thread
| Started by | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| First post | 2016-03-14 15:40 +0100 |
| Last post | 2016-03-14 23:30 +0100 |
| Articles | 6 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann <arnd@arndb.de> - 2016-03-14 15:40 +0100
Re: [PATCH 2/3] lpfc: fix misleading indentation Arnd Bergmann <arnd@arndb.de> - 2016-03-14 16:30 +0100
Re: [PATCH 2/3] lpfc: fix misleading indentation Hannes Reinecke <hare@suse.de> - 2016-03-14 16:30 +0100
Re: [PATCH 2/3] lpfc: fix misleading indentation Ewan Milne <emilne@redhat.com> - 2016-03-14 16:50 +0100
Re: [PATCH 2/3] lpfc: fix misleading indentation Hannes Reinecke <hare@suse.de> - 2016-03-14 16:30 +0100
Re: [PATCH 2/3] lpfc: fix misleading indentation Sebastian Herbszt <herbszt@gmx.de> - 2016-03-14 23:30 +0100
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-03-14 15:40 +0100 |
| Subject | [PATCH 2/3] lpfc: fix misleading indentation |
| Message-ID | <rcBP4-4Xz-33@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2016-03-14 16:30 +0100 |
| Message-ID | <rcCBs-5C6-7@gated-at.bofh.it> |
| In reply to | #1357301 |
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
[toc] | [prev] | [next] | [standalone]
| From | Hannes Reinecke <hare@suse.de> |
|---|---|
| Date | 2016-03-14 16:30 +0100 |
| Message-ID | <rcCBs-5C6-13@gated-at.bofh.it> |
| In reply to | #1357354 |
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)
[toc] | [prev] | [next] | [standalone]
| From | Ewan Milne <emilne@redhat.com> |
|---|---|
| Date | 2016-03-14 16:50 +0100 |
| Message-ID | <rcCUO-5IT-17@gated-at.bofh.it> |
| In reply to | #1357355 |
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
[toc] | [prev] | [next] | [standalone]
| From | Hannes Reinecke <hare@suse.de> |
|---|---|
| Date | 2016-03-14 16:30 +0100 |
| Message-ID | <rcCBs-5C6-9@gated-at.bofh.it> |
| In reply to | #1357301 |
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)
[toc] | [prev] | [next] | [standalone]
| From | Sebastian Herbszt <herbszt@gmx.de> |
|---|---|
| Date | 2016-03-14 23:30 +0100 |
| Message-ID | <rcJ9T-1vi-9@gated-at.bofh.it> |
| In reply to | #1357301 |
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>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web