* staging: comedi: non working tests on unsigned cmd->convert_arg @ 2009-04-21 13:18 Roel Kluin 2009-04-21 13:22 ` Roel Kluin 2009-04-22 16:50 ` Greg KH 0 siblings, 2 replies; 7+ messages in thread From: Roel Kluin @ 2009-04-21 13:18 UTC (permalink / raw) To: Greg KH; +Cc: lkml vi drivers/staging/comedi/comedi.h +342 and note that cmd->convert_arg in s626_ai_cmdtest() is unsigned. Should there maybe be a test: if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > MAX) (and what should MAX be then?) Otherwise you may want to apply the cleanup patch below Roel ------------------------------>8-------------8<--------------------------------- Remove redundant tests on unsigned Signed-off-by: Roel Kluin <roel.kluin@gmail.com> --- diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c index 30dec9d..928c4c6 100644 --- a/drivers/staging/comedi/drivers/s626.c +++ b/drivers/staging/comedi/drivers/s626.c @@ -1906,7 +1906,7 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s err++; } - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg < 0) { + if (cmd->convert_src == TRIG_EXT) { cmd->convert_arg = 0; err++; } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: staging: comedi: non working tests on unsigned cmd->convert_arg 2009-04-21 13:18 staging: comedi: non working tests on unsigned cmd->convert_arg Roel Kluin @ 2009-04-21 13:22 ` Roel Kluin 2009-04-22 16:50 ` Greg KH 1 sibling, 0 replies; 7+ messages in thread From: Roel Kluin @ 2009-04-21 13:22 UTC (permalink / raw) To: Greg KH; +Cc: lkml Roel Kluin wrote: > vi drivers/staging/comedi/comedi.h +342 > > and note that cmd->convert_arg in s626_ai_cmdtest() is unsigned. Should there > maybe be a test: > > if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > MAX) > > (and what should MAX be then?) The same question for cmd->scan_begin_arg and cmd->start_arg. Otherwise you may want to apply the cleanup patch below ------------------------------>8-------------8<--------------------------------- Remove redundant tests on unsigned Signed-off-by: Roel Kluin <roel.kluin@gmail.com> --- diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c index 30dec9d..637b0f1 100644 --- a/drivers/staging/comedi/drivers/s626.c +++ b/drivers/staging/comedi/drivers/s626.c @@ -1886,7 +1886,7 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s err++; } - if (cmd->start_src == TRIG_EXT && cmd->start_arg < 0) { + if (cmd->start_src == TRIG_EXT) { cmd->start_arg = 0; err++; } @@ -1896,7 +1896,7 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s err++; } - if (cmd->scan_begin_src == TRIG_EXT && cmd->scan_begin_arg < 0) { + if (cmd->scan_begin_src == TRIG_EXT) { cmd->scan_begin_arg = 0; err++; } @@ -1906,7 +1906,7 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s err++; } - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg < 0) { + if (cmd->convert_src == TRIG_EXT) { cmd->convert_arg = 0; err++; } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: staging: comedi: non working tests on unsigned cmd->convert_arg 2009-04-21 13:18 staging: comedi: non working tests on unsigned cmd->convert_arg Roel Kluin 2009-04-21 13:22 ` Roel Kluin @ 2009-04-22 16:50 ` Greg KH 2009-04-22 17:59 ` Ian Abbott [not found] ` <200904221411.13851.fmhess@speakeasy.net> 1 sibling, 2 replies; 7+ messages in thread From: Greg KH @ 2009-04-22 16:50 UTC (permalink / raw) To: Roel Kluin; +Cc: Greg KH, lkml, Ian Abbott, Frank Mori Hess, David Schleef On Tue, Apr 21, 2009 at 03:18:10PM +0200, Roel Kluin wrote: > vi drivers/staging/comedi/comedi.h +342 > > and note that cmd->convert_arg in s626_ai_cmdtest() is unsigned. Should there > maybe be a test: > > if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > MAX) > > (and what should MAX be then?) I don't know. Let's ask the comedi developers :) Frank, what do you think about this? > Otherwise you may want to apply the cleanup patch below > > Roel > ------------------------------>8-------------8<--------------------------------- > Remove redundant tests on unsigned > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> > --- > diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c > index 30dec9d..928c4c6 100644 > --- a/drivers/staging/comedi/drivers/s626.c > +++ b/drivers/staging/comedi/drivers/s626.c > @@ -1906,7 +1906,7 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s > err++; > } > > - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg < 0) { > + if (cmd->convert_src == TRIG_EXT) { > cmd->convert_arg = 0; > err++; > } ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: staging: comedi: non working tests on unsigned cmd->convert_arg 2009-04-22 16:50 ` Greg KH @ 2009-04-22 17:59 ` Ian Abbott [not found] ` <200904221411.13851.fmhess@speakeasy.net> 1 sibling, 0 replies; 7+ messages in thread From: Ian Abbott @ 2009-04-22 17:59 UTC (permalink / raw) To: Greg KH Cc: Roel Kluin, Greg KH, lkml, Ian Abbott, Frank Mori Hess, David Schleef On 22/04/09 17:50, Greg KH wrote: > On Tue, Apr 21, 2009 at 03:18:10PM +0200, Roel Kluin wrote: >> vi drivers/staging/comedi/comedi.h +342 >> >> and note that cmd->convert_arg in s626_ai_cmdtest() is unsigned. Should there >> maybe be a test: >> >> if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > MAX) >> >> (and what should MAX be then?) > > I don't know. Let's ask the comedi developers :) > > Frank, what do you think about this? Ian here. I'd just delete that 'if' statement entirely as the test is never true and it is followed by another 'if' statement that tests the maximum value. The similar 'if' statements that test cmd->start_arg < 0, and cmd->scan_begin_arg < 0 can also be removed. >> Otherwise you may want to apply the cleanup patch below >> >> Roel >> ------------------------------>8-------------8<--------------------------------- >> Remove redundant tests on unsigned >> >> Signed-off-by: Roel Kluin <roel.kluin@gmail.com> >> --- >> diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c >> index 30dec9d..928c4c6 100644 >> --- a/drivers/staging/comedi/drivers/s626.c >> +++ b/drivers/staging/comedi/drivers/s626.c >> @@ -1906,7 +1906,7 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s >> err++; >> } >> >> - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg < 0) { >> + if (cmd->convert_src == TRIG_EXT) { >> cmd->convert_arg = 0; >> err++; >> } That should be: - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg < 0) { - cmd->convert_arg = 0; - err++; - } -- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <200904221411.13851.fmhess@speakeasy.net>]
* Re: staging: comedi: non working tests on unsigned cmd->convert_arg [not found] ` <200904221411.13851.fmhess@speakeasy.net> @ 2009-04-23 2:08 ` Greg KH 2009-04-23 10:23 ` Roel Kluin 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2009-04-23 2:08 UTC (permalink / raw) To: fmhess; +Cc: Roel Kluin, Greg KH, lkml, Ian Abbott, David Schleef On Wed, Apr 22, 2009 at 02:11:10PM -0400, Frank Mori Hess wrote: > On Wednesday 22 April 2009, Greg KH wrote: > > On Tue, Apr 21, 2009 at 03:18:10PM +0200, Roel Kluin wrote: > > > vi drivers/staging/comedi/comedi.h +342 > > > > > > and note that cmd->convert_arg in s626_ai_cmdtest() is unsigned. > > > Should there maybe be a test: > > > > > > if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > MAX) > > > > > > (and what should MAX be then?) > > > > I don't know. Let's ask the comedi developers :) > > > > Frank, what do you think about this? > > > It looks like there already is such a test, right below the useless one the > patch removes. Ok, thanks for checking. Roel, care to resend these patches then? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: staging: comedi: non working tests on unsigned cmd->convert_arg 2009-04-23 2:08 ` Greg KH @ 2009-04-23 10:23 ` Roel Kluin 2009-04-23 10:53 ` Ian Abbott 0 siblings, 1 reply; 7+ messages in thread From: Roel Kluin @ 2009-04-23 10:23 UTC (permalink / raw) To: Greg KH; +Cc: fmhess, Greg KH, lkml, Ian Abbott, David Schleef Greg KH wrote: > On Wed, Apr 22, 2009 at 02:11:10PM -0400, Frank Mori Hess wrote: >> On Wednesday 22 April 2009, Greg KH wrote: >>> On Tue, Apr 21, 2009 at 03:18:10PM +0200, Roel Kluin wrote: >>>> vi drivers/staging/comedi/comedi.h +342 >>>> >>>> and note that cmd->convert_arg in s626_ai_cmdtest() is unsigned. >>>> Should there maybe be a test: >>>> >>>> if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > MAX) >>>> >>>> (and what should MAX be then?) >>> I don't know. Let's ask the comedi developers :) >>> >>> Frank, what do you think about this? >> >> It looks like there already is such a test, right below the useless one the >> patch removes. > > Ok, thanks for checking. Roel, care to resend these patches then? > > thanks, > > greg k-h > So you don't want to set cmd->*_arg to 0 when beyond 39 or below 0? ------------------------------>8-------------8<--------------------------------- Remove tests for negative unsigned. Signed-off-by: Roel Kluin <roel.kluin@gmail.com> --- diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c index 30dec9d..a48b244 100644 --- a/drivers/staging/comedi/drivers/s626.c +++ b/drivers/staging/comedi/drivers/s626.c @@ -1886,31 +1886,16 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s err++; } - if (cmd->start_src == TRIG_EXT && cmd->start_arg < 0) { - cmd->start_arg = 0; - err++; - } - if (cmd->start_src == TRIG_EXT && cmd->start_arg > 39) { cmd->start_arg = 39; err++; } - if (cmd->scan_begin_src == TRIG_EXT && cmd->scan_begin_arg < 0) { - cmd->scan_begin_arg = 0; - err++; - } - if (cmd->scan_begin_src == TRIG_EXT && cmd->scan_begin_arg > 39) { cmd->scan_begin_arg = 39; err++; } - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg < 0) { - cmd->convert_arg = 0; - err++; - } - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > 39) { cmd->convert_arg = 39; err++; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: staging: comedi: non working tests on unsigned cmd->convert_arg 2009-04-23 10:23 ` Roel Kluin @ 2009-04-23 10:53 ` Ian Abbott 0 siblings, 0 replies; 7+ messages in thread From: Ian Abbott @ 2009-04-23 10:53 UTC (permalink / raw) To: Roel Kluin; +Cc: Greg KH, fmhess, Greg KH, lkml, Ian Abbott, David Schleef Roel Kluin wrote: > Greg KH wrote: >> On Wed, Apr 22, 2009 at 02:11:10PM -0400, Frank Mori Hess wrote: >>> On Wednesday 22 April 2009, Greg KH wrote: >>>> On Tue, Apr 21, 2009 at 03:18:10PM +0200, Roel Kluin wrote: >>>>> vi drivers/staging/comedi/comedi.h +342 >>>>> >>>>> and note that cmd->convert_arg in s626_ai_cmdtest() is unsigned. >>>>> Should there maybe be a test: >>>>> >>>>> if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > MAX) >>>>> >>>>> (and what should MAX be then?) >>>> I don't know. Let's ask the comedi developers :) >>>> >>>> Frank, what do you think about this? >>> It looks like there already is such a test, right below the useless one the >>> patch removes. >> Ok, thanks for checking. Roel, care to resend these patches then? >> >> thanks, >> >> greg k-h >> > So you don't want to set cmd->*_arg to 0 when beyond 39 or below 0? Well setting cmd->*_arg to 0 when beyond 39 would be a change of functionality, so that's a no-no for a cleanup patch. And it would never be below 0 because it's unsigned, so those 'if' statements in the original were redundant. > ------------------------------>8-------------8<--------------------------------- > Remove tests for negative unsigned. > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> > --- > diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c > index 30dec9d..a48b244 100644 > --- a/drivers/staging/comedi/drivers/s626.c > +++ b/drivers/staging/comedi/drivers/s626.c > @@ -1886,31 +1886,16 @@ static int s626_ai_cmdtest(struct comedi_device *dev, struct comedi_subdevice *s > err++; > } > > - if (cmd->start_src == TRIG_EXT && cmd->start_arg < 0) { > - cmd->start_arg = 0; > - err++; > - } > - > if (cmd->start_src == TRIG_EXT && cmd->start_arg > 39) { > cmd->start_arg = 39; > err++; > } > > - if (cmd->scan_begin_src == TRIG_EXT && cmd->scan_begin_arg < 0) { > - cmd->scan_begin_arg = 0; > - err++; > - } > - > if (cmd->scan_begin_src == TRIG_EXT && cmd->scan_begin_arg > 39) { > cmd->scan_begin_arg = 39; > err++; > } > > - if (cmd->convert_src == TRIG_EXT && cmd->convert_arg < 0) { > - cmd->convert_arg = 0; > - err++; > - } > - > if (cmd->convert_src == TRIG_EXT && cmd->convert_arg > 39) { > cmd->convert_arg = 39; > err++; -- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-04-23 10:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-21 13:18 staging: comedi: non working tests on unsigned cmd->convert_arg Roel Kluin
2009-04-21 13:22 ` Roel Kluin
2009-04-22 16:50 ` Greg KH
2009-04-22 17:59 ` Ian Abbott
[not found] ` <200904221411.13851.fmhess@speakeasy.net>
2009-04-23 2:08 ` Greg KH
2009-04-23 10:23 ` Roel Kluin
2009-04-23 10:53 ` Ian Abbott
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.