* [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions
@ 2016-02-10 8:59 Amitoj Kaur Chawla
2016-02-10 9:09 ` [Outreachy kernel] " Julia Lawall
2016-02-12 3:45 ` Greg KH
0 siblings, 2 replies; 6+ messages in thread
From: Amitoj Kaur Chawla @ 2016-02-10 8:59 UTC (permalink / raw)
To: outreachy-kernel
Merge if conditions with the same statements.
Found using Coccinelle. The semantic patch used to find this is as
follows:
//<smpl>
@@
statement S;
expression e,x;
@@
* if(e)
(
{ ... return ...; }
&
S
)
* if(x)
(
{ ... return ...; }
&
S
)
//</smpl>
Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
drivers/staging/comedi/drivers/pcmuio.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c
index 7ea8130..d303b48 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -319,10 +319,7 @@ static void pcmuio_handle_intr_subdev(struct comedi_device *dev,
spin_lock_irqsave(&chip->spinlock, flags);
- if (!chip->active)
- goto done;
-
- if (!(triggered & chip->enabled_mask))
+ if (!(chip->active && (triggered & chip->enabled_mask)))
goto done;
for (i = 0; i < cmd->chanlist_len; i++) {
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions
2016-02-10 8:59 [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions Amitoj Kaur Chawla
@ 2016-02-10 9:09 ` Julia Lawall
2016-02-10 9:12 ` Amitoj Kaur Chawla
2016-02-12 3:45 ` Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2016-02-10 9:09 UTC (permalink / raw)
To: Amitoj Kaur Chawla; +Cc: outreachy-kernel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1792 bytes --]
On Wed, 10 Feb 2016, Amitoj Kaur Chawla wrote:
> Merge if conditions with the same statements.
>
> Found using Coccinelle. The semantic patch used to find this is as
> follows:
>
> //<smpl>
> @@
> statement S;
> expression e,x;
> @@
>
> * if(e)
> (
> { ... return ...; }
> &
> S
> )
>
> * if(x)
> (
> { ... return ...; }
> &
> S
> )
> //</smpl>
>
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> ---
> drivers/staging/comedi/drivers/pcmuio.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c
> index 7ea8130..d303b48 100644
> --- a/drivers/staging/comedi/drivers/pcmuio.c
> +++ b/drivers/staging/comedi/drivers/pcmuio.c
> @@ -319,10 +319,7 @@ static void pcmuio_handle_intr_subdev(struct comedi_device *dev,
>
> spin_lock_irqsave(&chip->spinlock, flags);
>
> - if (!chip->active)
> - goto done;
> -
> - if (!(triggered & chip->enabled_mask))
> + if (!(chip->active && (triggered & chip->enabled_mask)))
I�m not sure that converting to only one ! in this way is a good idea.
!X || !Y could be more readable. Here there are a lot of parentheses to
sort out.
julia
> goto done;
>
> for (i = 0; i < cmd->chanlist_len; i++) {
> --
> 1.9.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160210085942.GA14651%40amitoj-Inspiron-3542.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions
2016-02-10 9:09 ` [Outreachy kernel] " Julia Lawall
@ 2016-02-10 9:12 ` Amitoj Kaur Chawla
2016-02-10 9:18 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Amitoj Kaur Chawla @ 2016-02-10 9:12 UTC (permalink / raw)
To: Julia Lawall; +Cc: outreachy-kernel
On Wed, Feb 10, 2016 at 2:39 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> - if (!chip->active)
>> - goto done;
>> -
>> - if (!(triggered & chip->enabled_mask))
>> + if (!(chip->active && (triggered & chip->enabled_mask)))
>
> I´m not sure that converting to only one ! in this way is a good idea.
> !X || !Y could be more readable. Here there are a lot of parentheses to
> sort out.
>
> julia
>
I wasn't very sure about this either whether I should make it !(X &&
Y) or !X || !Y.
Since Patch 5/6 also does a similar thing, would you like me to change
to !X || !Y for both?
Amitoj
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions
2016-02-10 9:12 ` Amitoj Kaur Chawla
@ 2016-02-10 9:18 ` Julia Lawall
2016-02-10 9:19 ` Amitoj Kaur Chawla
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2016-02-10 9:18 UTC (permalink / raw)
To: Amitoj Kaur Chawla; +Cc: outreachy-kernel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1335 bytes --]
On Wed, 10 Feb 2016, Amitoj Kaur Chawla wrote:
> On Wed, Feb 10, 2016 at 2:39 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >> - if (!chip->active)
> >> - goto done;
> >> -
> >> - if (!(triggered & chip->enabled_mask))
> >> + if (!(chip->active && (triggered & chip->enabled_mask)))
> >
> > I´m not sure that converting to only one ! in this way is a good idea.
> > !X || !Y could be more readable. Here there are a lot of parentheses to
> > sort out.
> >
> > julia
> >
>
> I wasn't very sure about this either whether I should make it !(X &&
> Y) or !X || !Y.
> Since Patch 5/6 also does a similar thing, would you like me to change
> to !X || !Y for both?
Yes, for all cases. I didn´t want to make the same comemnt more than once
:)
julia
>
> Amitoj
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CA%2B5yK5F1nNgDLCJrFm3CrgOjCe3OJUutMrQ5%2B2R_FOrz3g0wzQ%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions
2016-02-10 9:18 ` Julia Lawall
@ 2016-02-10 9:19 ` Amitoj Kaur Chawla
0 siblings, 0 replies; 6+ messages in thread
From: Amitoj Kaur Chawla @ 2016-02-10 9:19 UTC (permalink / raw)
To: Julia Lawall; +Cc: outreachy-kernel
On Wed, Feb 10, 2016 at 2:48 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> I wasn't very sure about this either whether I should make it !(X &&
>> Y) or !X || !Y.
>> Since Patch 5/6 also does a similar thing, would you like me to change
>> to !X || !Y for both?
>
> Yes, for all cases. I didn´t want to make the same comemnt more than once
> :)
>
> julia
>
Okay!
Amitoj
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Outreachy kernel] [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions
2016-02-10 8:59 [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions Amitoj Kaur Chawla
2016-02-10 9:09 ` [Outreachy kernel] " Julia Lawall
@ 2016-02-12 3:45 ` Greg KH
1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2016-02-12 3:45 UTC (permalink / raw)
To: Amitoj Kaur Chawla; +Cc: outreachy-kernel
On Wed, Feb 10, 2016 at 02:29:42PM +0530, Amitoj Kaur Chawla wrote:
> Merge if conditions with the same statements.
>
> Found using Coccinelle. The semantic patch used to find this is as
> follows:
>
> //<smpl>
> @@
> statement S;
> expression e,x;
> @@
>
> * if(e)
> (
> { ... return ...; }
> &
> S
> )
>
> * if(x)
> (
> { ... return ...; }
> &
> S
> )
> //</smpl>
>
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> ---
> drivers/staging/comedi/drivers/pcmuio.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c
> index 7ea8130..d303b48 100644
> --- a/drivers/staging/comedi/drivers/pcmuio.c
> +++ b/drivers/staging/comedi/drivers/pcmuio.c
> @@ -319,10 +319,7 @@ static void pcmuio_handle_intr_subdev(struct comedi_device *dev,
>
> spin_lock_irqsave(&chip->spinlock, flags);
>
> - if (!chip->active)
> - goto done;
> -
> - if (!(triggered & chip->enabled_mask))
> + if (!(chip->active && (triggered & chip->enabled_mask)))
> goto done;
This is now harder to read, sorry, I don't like these at all.
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-12 3:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-10 8:59 [PATCH 6/6] staging: comedi: drivers: pcmuio: Merge if conditions Amitoj Kaur Chawla
2016-02-10 9:09 ` [Outreachy kernel] " Julia Lawall
2016-02-10 9:12 ` Amitoj Kaur Chawla
2016-02-10 9:18 ` Julia Lawall
2016-02-10 9:19 ` Amitoj Kaur Chawla
2016-02-12 3:45 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.