* 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
* 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.