All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks
@ 2020-03-14 16:51 Deepak R Varma
  2020-03-14 16:57 ` [Outreachy kernel] " Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Deepak R Varma @ 2020-03-14 16:51 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: gregkh, daniel.baluta, kieran.bingham, abbotti, hsweeten

Remove two if# 0 directive code blocks. The first commented code block
is redundant too; whereas the other code block is never used. Problem
detected by checkpatch script.

Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
---
 drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c
index 68ad9676f962..0bca7d752015 100644
--- a/drivers/staging/comedi/drivers/ni_atmio16d.c
+++ b/drivers/staging/comedi/drivers/ni_atmio16d.c
@@ -266,19 +266,9 @@ static int atmio16d_ai_cmdtest(struct comedi_device *dev,
 	if (cmd->scan_begin_src == TRIG_FOLLOW) {
 		/* internal trigger */
 		err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
-	} else {
-#if 0
-		/* external trigger */
-		/* should be level/edge, hi/lo specification here */
-		err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
-#endif
 	}
 
 	err |= comedi_check_trigger_arg_min(&cmd->convert_arg, 10000);
-#if 0
-	err |= comedi_check_trigger_arg_max(&cmd->convert_arg, SLOWEST_TIMER);
-#endif
-
 	err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg,
 					   cmd->chanlist_len);
 
-- 
2.17.1



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

* Re: [Outreachy kernel] [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks
  2020-03-14 16:51 [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks Deepak R Varma
@ 2020-03-14 16:57 ` Julia Lawall
  2020-03-14 17:04   ` Deepak Varma
  0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2020-03-14 16:57 UTC (permalink / raw)
  To: Deepak R Varma
  Cc: outreachy-kernel, gregkh, daniel.baluta, kieran.bingham, abbotti,
	hsweeten



On Sat, 14 Mar 2020, Deepak R Varma wrote:

> Remove two if# 0 directive code blocks. The first commented code block

#if

> is redundant too; whereas the other code block is never used. Problem

Why do you say that the first one is redundant?

julia

> detected by checkpatch script.
>
> Signed-off-by: Deepak R Varma <mh12gx2825@gmail.com>
> ---
>  drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c b/drivers/staging/comedi/drivers/ni_atmio16d.c
> index 68ad9676f962..0bca7d752015 100644
> --- a/drivers/staging/comedi/drivers/ni_atmio16d.c
> +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c
> @@ -266,19 +266,9 @@ static int atmio16d_ai_cmdtest(struct comedi_device *dev,
>  	if (cmd->scan_begin_src == TRIG_FOLLOW) {
>  		/* internal trigger */
>  		err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
> -	} else {
> -#if 0
> -		/* external trigger */
> -		/* should be level/edge, hi/lo specification here */
> -		err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
> -#endif
>  	}
>
>  	err |= comedi_check_trigger_arg_min(&cmd->convert_arg, 10000);
> -#if 0
> -	err |= comedi_check_trigger_arg_max(&cmd->convert_arg, SLOWEST_TIMER);
> -#endif
> -
>  	err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg,
>  					   cmd->chanlist_len);
>
> --
> 2.17.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 view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20200314165130.GA18939%40deeUbuntu.
>


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

* Re: [Outreachy kernel] [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks
  2020-03-14 16:57 ` [Outreachy kernel] " Julia Lawall
@ 2020-03-14 17:04   ` Deepak Varma
  2020-03-14 17:10     ` Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Deepak Varma @ 2020-03-14 17:04 UTC (permalink / raw)
  To: outreachy-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2341 bytes --]



On Saturday, 14 March 2020 22:27:15 UTC+5:30, Julia Lawall wrote:
>
>
>
> On Sat, 14 Mar 2020, Deepak R Varma wrote: 
>
> > Remove two if# 0 directive code blocks. The first commented code block 
>
> #if 
>
> > is redundant too; whereas the other code block is never used. Problem 
>
> Why do you say that the first one is redundant? 
>
> julia 
>

The instructions in if and else block are exactly the same. so, if or else, 
you are going to run the same instruction. Let me know if I misunderstood 
or misquoted.

Thank you.


> > detected by checkpatch script. 
> > 
> > Signed-off-by: Deepak R Varma <mh12g...@gmail.com <javascript:>> 
> > --- 
> >  drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ---------- 
> >  1 file changed, 10 deletions(-) 
> > 
> > diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c 
> b/drivers/staging/comedi/drivers/ni_atmio16d.c 
> > index 68ad9676f962..0bca7d752015 100644 
> > --- a/drivers/staging/comedi/drivers/ni_atmio16d.c 
> > +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c 
> > @@ -266,19 +266,9 @@ static int atmio16d_ai_cmdtest(struct comedi_device 
> *dev, 
> >          if (cmd->scan_begin_src == TRIG_FOLLOW) { 
> >                  /* internal trigger */ 
> >                  err |= 
> comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0); 
> > -        } else { 
> > -#if 0 
> > -                /* external trigger */ 
> > -                /* should be level/edge, hi/lo specification here */ 
> > -                err |= 
> comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0); 
> > -#endif 
> >          } 
> > 
> >          err |= comedi_check_trigger_arg_min(&cmd->convert_arg, 10000); 
> > -#if 0 
> > -        err |= comedi_check_trigger_arg_max(&cmd->convert_arg, 
> SLOWEST_TIMER); 
> > -#endif 
> > - 
> >          err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg, 
> >                                             cmd->chanlist_len); 
> > 
> > -- 
> > 2.17.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 outreach...@googlegroups.com <javascript:>. 
> > To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20200314165130.GA18939%40deeUbuntu. 
>
> > 
>

[-- Attachment #1.2: Type: text/html, Size: 3996 bytes --]

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

* Re: [Outreachy kernel] [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks
  2020-03-14 17:04   ` Deepak Varma
@ 2020-03-14 17:10     ` Julia Lawall
  2020-03-14 17:28       ` Deepak Varma
  0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2020-03-14 17:10 UTC (permalink / raw)
  To: Deepak Varma; +Cc: outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 3754 bytes --]



On Sat, 14 Mar 2020, Deepak Varma wrote:

>
>
> On Saturday, 14 March 2020 22:27:15 UTC+5:30, Julia Lawall wrote:
>
>
>       On Sat, 14 Mar 2020, Deepak R Varma wrote:
>
>       > Remove two if# 0 directive code blocks. The first commented
>       code block
>
>       #if
>
>       > is redundant too; whereas the other code block is never used.
>       Problem
>
>       Why do you say that the first one is redundant?
>
>       julia
>
>
> The instructions in if and else block are exactly the same. so, if or else,
> you are going to run the same instruction. Let me know if I misunderstood or
> misquoted.

Since one of them is under an #if 0, the current effect is quite
different.  Only the statement in the then branch will be executed.

Redundant would be eg:

x = 0;
x = 0;

There is no point in having the second line, because x is already 0.

Even without the #if 0, if you have:

if (x)
  y = 0;
else y = 0;

It doesn't seem reasonable to say that the second y = 0 is redundant. It
is the if that is is not necessary.  But once you have the if, you need
both the then and else branches.

julia

>
> Thank you.
>
>
>       > detected by checkpatch script.
>       >
>       > Signed-off-by: Deepak R Varma <mh12g...@gmail.com>
>       > ---
>       >  drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ----------
>       >  1 file changed, 10 deletions(-)
>       >
>       > diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c
>       b/drivers/staging/comedi/drivers/ni_atmio16d.c
>       > index 68ad9676f962..0bca7d752015 100644
>       > --- a/drivers/staging/comedi/drivers/ni_atmio16d.c
>       > +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c
>       > @@ -266,19 +266,9 @@ static int atmio16d_ai_cmdtest(struct
>       comedi_device *dev,
>       >          if (cmd->scan_begin_src == TRIG_FOLLOW) {
>       >                  /* internal trigger */
>       >                  err |=
>       comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
>       > -        } else {
>       > -#if 0
>       > -                /* external trigger */
>       > -                /* should be level/edge, hi/lo specification
>       here */
>       > -                err |=
>       comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
>       > -#endif
>       >          }
>       >
>       >          err |=
>       comedi_check_trigger_arg_min(&cmd->convert_arg, 10000);
>       > -#if 0
>       > -        err |=
>       comedi_check_trigger_arg_max(&cmd->convert_arg, SLOWEST_TIMER);
>       > -#endif
>       > -
>       >          err |=
>       comedi_check_trigger_arg_is(&cmd->scan_end_arg,
>       >                                            
>       cmd->chanlist_len);
>       >
>       > --
>       > 2.17.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 outreach...@googlegroups.com.
>       > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20200314165130.GA18939%4
>       0deeUbuntu.
>       >
>
> --
> 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 view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/7842c46c-710b-4a2a-bf46-
> 396ed319e2a6%40googlegroups.com.
>
>

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

* Re: [Outreachy kernel] [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks
  2020-03-14 17:10     ` Julia Lawall
@ 2020-03-14 17:28       ` Deepak Varma
  2020-03-14 17:31         ` Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Deepak Varma @ 2020-03-14 17:28 UTC (permalink / raw)
  To: outreachy-kernel


[-- Attachment #1.1: Type: text/plain, Size: 4448 bytes --]



On Saturday, 14 March 2020 22:41:02 UTC+5:30, Julia Lawall wrote:
>
>
>
> On Sat, 14 Mar 2020, Deepak Varma wrote: 
>
> > 
> > 
> > On Saturday, 14 March 2020 22:27:15 UTC+5:30, Julia Lawall wrote: 
> > 
> > 
> >       On Sat, 14 Mar 2020, Deepak R Varma wrote: 
> > 
> >       > Remove two if# 0 directive code blocks. The first commented 
> >       code block 
> > 
> >       #if 
> > 
> >       > is redundant too; whereas the other code block is never used. 
> >       Problem 
> > 
> >       Why do you say that the first one is redundant? 
> > 
> >       julia 
> > 
> > 
> > The instructions in if and else block are exactly the same. so, if or 
> else, 
> > you are going to run the same instruction. Let me know if I 
> misunderstood or 
> > misquoted. 
>
> Since one of them is under an #if 0, the current effect is quite 
> different.  Only the statement in the then branch will be executed. 
>
> Redundant would be eg: 
>
> x = 0; 
> x = 0; 
>
> There is no point in having the second line, because x is already 0. 
>
> Even without the #if 0, if you have: 
>
> if (x) 
>   y = 0; 
> else y = 0; 
>
> It doesn't seem reasonable to say that the second y = 0 is redundant. It 
> is the if that is is not necessary.  But once you have the if, you need 
> both the then and else branches. 
>
> julia 
>

Agreed. if(x) becomes redundant when #if 0 becomes #if 1. I was trying to 
make the same point that even if the #if 0 block is turned on, it will 
still have the same behavior with the if(x) .. else in place. But, I though 
the else part will be redundant, but it was wrong. The if part would be 
redundant. I got your point. Well explained. Thank you!

Would you suggest I reword the patch description and send in v2? Is the 
rest looking good to you?

Deepak.


> > 
> > Thank you. 
> > 
> > 
> >       > detected by checkpatch script. 
> >       > 
> >       > Signed-off-by: Deepak R Varma <mh12g...@gmail.com> 
> >       > --- 
> >       >  drivers/staging/comedi/drivers/ni_atmio16d.c | 10 ---------- 
> >       >  1 file changed, 10 deletions(-) 
> >       > 
> >       > diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c 
> >       b/drivers/staging/comedi/drivers/ni_atmio16d.c 
> >       > index 68ad9676f962..0bca7d752015 100644 
> >       > --- a/drivers/staging/comedi/drivers/ni_atmio16d.c 
> >       > +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c 
> >       > @@ -266,19 +266,9 @@ static int atmio16d_ai_cmdtest(struct 
> >       comedi_device *dev, 
> >       >          if (cmd->scan_begin_src == TRIG_FOLLOW) { 
> >       >                  /* internal trigger */ 
> >       >                  err |= 
> >       comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0); 
> >       > -        } else { 
> >       > -#if 0 
> >       > -                /* external trigger */ 
> >       > -                /* should be level/edge, hi/lo specification 
> >       here */ 
> >       > -                err |= 
> >       comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0); 
> >       > -#endif 
> >       >          } 
> >       > 
> >       >          err |= 
> >       comedi_check_trigger_arg_min(&cmd->convert_arg, 10000); 
> >       > -#if 0 
> >       > -        err |= 
> >       comedi_check_trigger_arg_max(&cmd->convert_arg, SLOWEST_TIMER); 
> >       > -#endif 
> >       > - 
> >       >          err |= 
> >       comedi_check_trigger_arg_is(&cmd->scan_end_arg, 
> >       >                                             
> >       cmd->chanlist_len); 
> >       > 
> >       > -- 
> >       > 2.17.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 outreach...@googlegroups.com. 
> >       > To view this discussion on the web visithttps://
> groups.google.com/d/msgid/outreachy-kernel/20200314165130.GA18939%4 
> >       0deeUbuntu. 
> >       > 
> > 
> > -- 
> > 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 outreach...@googlegroups.com <javascript:>. 
> > To view this discussion on the web visithttps://
> groups.google.com/d/msgid/outreachy-kernel/7842c46c-710b-4a2a-bf46- 
> > 396ed319e2a6%40googlegroups.com. 
> > 
> >


[-- Attachment #1.2: Type: text/html, Size: 7308 bytes --]

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

* Re: [Outreachy kernel] [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks
  2020-03-14 17:28       ` Deepak Varma
@ 2020-03-14 17:31         ` Julia Lawall
  0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2020-03-14 17:31 UTC (permalink / raw)
  To: Deepak Varma; +Cc: outreachy-kernel

[-- Attachment #1: Type: text/plain, Size: 5933 bytes --]



On Sat, 14 Mar 2020, Deepak Varma wrote:

>
>
> On Saturday, 14 March 2020 22:41:02 UTC+5:30, Julia Lawall wrote:
>
>
>       On Sat, 14 Mar 2020, Deepak Varma wrote:
>
>       >
>       >
>       > On Saturday, 14 March 2020 22:27:15 UTC+5:30, Julia Lawall
>       wrote:
>       >
>       >
>       >       On Sat, 14 Mar 2020, Deepak R Varma wrote:
>       >
>       >       > Remove two if# 0 directive code blocks. The first
>       commented
>       >       code block
>       >
>       >       #if
>       >
>       >       > is redundant too; whereas the other code block is
>       never used.
>       >       Problem
>       >
>       >       Why do you say that the first one is redundant?
>       >
>       >       julia
>       >
>       >
>       > The instructions in if and else block are exactly the same.
>       so, if or else,
>       > you are going to run the same instruction. Let me know if I
>       misunderstood or
>       > misquoted.
>
>       Since one of them is under an #if 0, the current effect is quite
>       different.  Only the statement in the then branch will be
>       executed.
>
>       Redundant would be eg:
>
>       x = 0;
>       x = 0;
>
>       There is no point in having the second line, because x is
>       already 0.
>
>       Even without the #if 0, if you have:
>
>       if (x)
>         y = 0;
>       else y = 0;
>
>       It doesn't seem reasonable to say that the second y = 0 is
>       redundant. It
>       is the if that is is not necessary.  But once you have the if,
>       you need
>       both the then and else branches.
>
>       julia
>
>
> Agreed. if(x) becomes redundant when #if 0 becomes #if 1. I was trying to
> make the same point that even if the #if 0 block is turned on, it will still
> have the same behavior with the if(x) .. else in place. But, I though the
> else part will be redundant, but it was wrong. The if part would be
> redundant. I got your point. Well explained. Thank you!
>
> Would you suggest I reword the patch description and send in v2? Is the rest
> looking good to you?

Yes for both.

julia

> Deepak.
>
>
>       >
>       > Thank you.
>       >
>       >
>       >       > detected by checkpatch script.
>       >       >
>       >       > Signed-off-by: Deepak R Varma <mh12g...@gmail.com>
>       >       > ---
>       >       >  drivers/staging/comedi/drivers/ni_atmio16d.c | 10
>       ----------
>       >       >  1 file changed, 10 deletions(-)
>       >       >
>       >       > diff --git
>       a/drivers/staging/comedi/drivers/ni_atmio16d.c
>       >       b/drivers/staging/comedi/drivers/ni_atmio16d.c
>       >       > index 68ad9676f962..0bca7d752015 100644
>       >       > --- a/drivers/staging/comedi/drivers/ni_atmio16d.c
>       >       > +++ b/drivers/staging/comedi/drivers/ni_atmio16d.c
>       >       > @@ -266,19 +266,9 @@ static int
>       atmio16d_ai_cmdtest(struct
>       >       comedi_device *dev,
>       >       >          if (cmd->scan_begin_src == TRIG_FOLLOW) {
>       >       >                  /* internal trigger */
>       >       >                  err |=
>       >       comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
>       >       > -        } else {
>       >       > -#if 0
>       >       > -                /* external trigger */
>       >       > -                /* should be level/edge, hi/lo
>       specification
>       >       here */
>       >       > -                err |=
>       >       comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
>       >       > -#endif
>       >       >          }
>       >       >
>       >       >          err |=
>       >       comedi_check_trigger_arg_min(&cmd->convert_arg, 10000);
>       >       > -#if 0
>       >       > -        err |=
>       >       comedi_check_trigger_arg_max(&cmd->convert_arg,
>       SLOWEST_TIMER);
>       >       > -#endif
>       >       > -
>       >       >          err |=
>       >       comedi_check_trigger_arg_is(&cmd->scan_end_arg,
>       >       >                                            
>       >       cmd->chanlist_len);
>       >       >
>       >       > --
>       >       > 2.17.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 outreach...@googlegroups.com.
>       >       > To view this discussion on the webvisithttps://groups.google.com/d/msgid/outreachy-kernel/20200314165130.GA18
>       939%4
>       >       0deeUbuntu.
>       >       >
>       >
>       > --
>       > 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 outreach...@googlegroups.com.
>       > To view this discussion on the webvisithttps://groups.google.com/d/msgid/outreachy-kernel/7842c46c-710b-4a2a-
>       bf46-
>       > 396ed319e2a6%40googlegroups.com.
>       >
>       >
>
> --
> 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 view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/7731265e-42a7-4c67-a0f5-
> 245cecbecc0b%40googlegroups.com.
>
>

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

end of thread, other threads:[~2020-03-14 17:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-14 16:51 [PATCH] staging: comedi: ni_atmio16d: remove commented code blocks Deepak R Varma
2020-03-14 16:57 ` [Outreachy kernel] " Julia Lawall
2020-03-14 17:04   ` Deepak Varma
2020-03-14 17:10     ` Julia Lawall
2020-03-14 17:28       ` Deepak Varma
2020-03-14 17:31         ` Julia Lawall

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.