Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1174776 > unrolled thread
| Started by | Sohny Thomas <sohnythomas@zoho.com> |
|---|---|
| First post | 2015-06-30 23:40 +0200 |
| Last post | 2015-07-01 10:40 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] Staging: unisys: virtpci: fixed a brace coding style issue Sohny Thomas <sohnythomas@zoho.com> - 2015-06-30 23:40 +0200
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue Sohny Thomas <sohnythomas@zoho.com> - 2015-07-01 09:40 +0200
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue Julia Lawall <julia.lawall@lip6.fr> - 2015-07-01 10:10 +0200
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-07-01 10:10 +0200
Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue Sohny Thomas <sohnythomas@zoho.com> - 2015-07-01 10:40 +0200
| From | Sohny Thomas <sohnythomas@zoho.com> |
|---|---|
| Date | 2015-06-30 23:40 +0200 |
| Subject | [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue |
| Message-ID | <pHbq1-5IH-3@gated-at.bofh.it> |
FIX 2 unnecessary braces found by checkpatch.pl
Signed-off-by: Sohny Thomas <sohnythomas@zoho.com>
---
drivers/staging/unisys/virtpci/virtpci.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/unisys/virtpci/virtpci.c b/drivers/staging/unisys/virtpci/virtpci.c
index d5ad017..f3674de 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -190,9 +190,10 @@ static int write_vbus_chp_info(struct spar_vbus_channel_protocol *chan,
return -1;
off = sizeof(struct channel_header) + chan->hdr_info.chp_info_offset;
- if (chan->hdr_info.chp_info_offset == 0) {
+
+ if (chan->hdr_info.chp_info_offset == 0)
return -1;
- }
+
memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
return 0;
}
@@ -484,10 +485,10 @@ static int delete_vhba(struct del_virt_guestpart *delparams)
i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
&scsi.wwnn, NULL);
- if (i) {
+ if (i)
return 1;
- }
- return 0;
+ else
+ return 0;
}
/* deletes a vnic
--
--
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/
[toc] | [next] | [standalone]
| From | Sohny Thomas <sohnythomas@zoho.com> |
|---|---|
| Date | 2015-07-01 09:40 +0200 |
| Subject | Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue |
| Message-ID | <pHkMF-2aU-13@gated-at.bofh.it> |
| In reply to | #1174776 |
Thanks for review, my answers inline
On 01-07-2015 12:27, Sudip Mukherjee wrote:
> On Wed, Jul 01, 2015 at 03:05:45AM +0530, Sohny Thomas wrote:
>>
>> FIX 2 unnecessary braces found by checkpatch.pl
>>
>> Signed-off-by: Sohny Thomas <sohnythomas@zoho.com>
>> ---
>> drivers/staging/unisys/virtpci/virtpci.c | 11 ++++++-----
>> 1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/unisys/virtpci/virtpci.c b/drivers/staging/unisys/virtpci/virtpci.c
>> index d5ad017..f3674de 100644
>> --- a/drivers/staging/unisys/virtpci/virtpci.c
>> +++ b/drivers/staging/unisys/virtpci/virtpci.c
>> @@ -190,9 +190,10 @@ static int write_vbus_chp_info(struct spar_vbus_channel_protocol *chan,
>> return -1;
>>
>> off = sizeof(struct channel_header) + chan->hdr_info.chp_info_offset;
>> - if (chan->hdr_info.chp_info_offset == 0) {
>> +
>> + if (chan->hdr_info.chp_info_offset == 0)
>> return -1;
>> - }
>> +
> why you are inserting new line here?
I did it so that its readable, will remove it if not required
>
>> memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
>> return 0;
>> }
>> @@ -484,10 +485,10 @@ static int delete_vhba(struct del_virt_guestpart *delparams)
>>
>> i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
>> &scsi.wwnn, NULL);
>> - if (i) {
>> + if (i)
>> return 1;
>> - }
>> - return 0;
>> + else
>> + return 0;
> No, now this will introduce a new checkpatch warning that "else is not
> required after return". why did you introduce this "else"?
I did this so that the code is more readable and understandable, I
checked and checkpatch didn't call this out , so its clean.
Otherwise the above code looks like this
if(i)
return 1;
return 0;
>
> regards
> sudip
>
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
--
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/
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2015-07-01 10:10 +0200 |
| Subject | Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue |
| Message-ID | <pHlfH-2Ay-11@gated-at.bofh.it> |
| In reply to | #1175039 |
On Wed, 1 Jul 2015, Sohny Thomas wrote:
> Thanks for review, my answers inline
>
> On 01-07-2015 12:27, Sudip Mukherjee wrote:
> > On Wed, Jul 01, 2015 at 03:05:45AM +0530, Sohny Thomas wrote:
> > >
> > > FIX 2 unnecessary braces found by checkpatch.pl
> > >
> > > Signed-off-by: Sohny Thomas <sohnythomas@zoho.com>
> > > ---
> > > drivers/staging/unisys/virtpci/virtpci.c | 11 ++++++-----
> > > 1 file changed, 6 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/staging/unisys/virtpci/virtpci.c
> > > b/drivers/staging/unisys/virtpci/virtpci.c
> > > index d5ad017..f3674de 100644
> > > --- a/drivers/staging/unisys/virtpci/virtpci.c
> > > +++ b/drivers/staging/unisys/virtpci/virtpci.c
> > > @@ -190,9 +190,10 @@ static int write_vbus_chp_info(struct
> > > spar_vbus_channel_protocol *chan,
> > > return -1;
> > >
> > > off = sizeof(struct channel_header) + chan->hdr_info.chp_info_offset;
> > > - if (chan->hdr_info.chp_info_offset == 0) {
> > > +
> > > + if (chan->hdr_info.chp_info_offset == 0)
> > > return -1;
> > > - }
> > > +
> > why you are inserting new line here?
> I did it so that its readable, will remove it if not required
> >
> > > memcpy(((u8 *)(chan)) + off, info, sizeof(*info));
> > > return 0;
> > > }
> > > @@ -484,10 +485,10 @@ static int delete_vhba(struct del_virt_guestpart
> > > *delparams)
> > >
> > > i = virtpci_device_del(NULL /*no parent bus */, VIRTHBA_TYPE,
> > > &scsi.wwnn, NULL);
> > > - if (i) {
> > > + if (i)
> > > return 1;
> > > - }
> > > - return 0;
> > > + else
> > > + return 0;
> > No, now this will introduce a new checkpatch warning that "else is not
> > required after return". why did you introduce this "else"?
> I did this so that the code is more readable and understandable, I checked and
> checkpatch didn't call this out , so its clean.
>
> Otherwise the above code looks like this
>
> if(i)
> return 1;
> return 0;
That looks fine.
I haven't looked at the code in detail. Is it normal that the return
values seem to be 0 1 and -1? Which values represent success and which
represent an error? It is nicer to have the errors under if and success
as a direct return at the end.
julia
--
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/
[toc] | [prev] | [next] | [standalone]
| From | Sudip Mukherjee <sudipm.mukherjee@gmail.com> |
|---|---|
| Date | 2015-07-01 10:10 +0200 |
| Subject | Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue |
| Message-ID | <pHlfI-2Ay-23@gated-at.bofh.it> |
| In reply to | #1175039 |
On Wed, Jul 01, 2015 at 01:06:48PM +0530, Sohny Thomas wrote: <snip> > >No, now this will introduce a new checkpatch warning that "else is not > >required after return". why did you introduce this "else"? > I did this so that the code is more readable and understandable, I > checked and checkpatch didn't call this out , so its clean. > > Otherwise the above code looks like this > > if(i) > return 1; > return 0; you should update your tree. virtpci folder has been deleted from unisys driver. As you are using an old tree, maybe that explains why checkpatch is not giving the error. regards sudip -- 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/
[toc] | [prev] | [next] | [standalone]
| From | Sohny Thomas <sohnythomas@zoho.com> |
|---|---|
| Date | 2015-07-01 10:40 +0200 |
| Subject | Re: [PATCH] Staging: unisys: virtpci: fixed a brace coding style issue |
| Message-ID | <pHlIK-2Mw-21@gated-at.bofh.it> |
| In reply to | #1175067 |
On Wednesday 01 July 2015 01:32 PM, Sudip Mukherjee wrote: > On Wed, Jul 01, 2015 at 01:06:48PM +0530, Sohny Thomas wrote: > <snip> >>> No, now this will introduce a new checkpatch warning that "else is not >>> required after return". why did you introduce this "else"? >> I did this so that the code is more readable and understandable, I >> checked and checkpatch didn't call this out , so its clean. >> >> Otherwise the above code looks like this >> >> if(i) >> return 1; >> return 0; > you should update your tree. virtpci folder has been deleted from > unisys driver. > As you are using an old tree, maybe that explains why checkpatch is not > giving the error. This is from linux-stable branch and I updated it just yesterday, so looks like the folders still there > > regards > sudip > -- 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/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web