* Re: [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
2014-02-13 3:29 [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c Chase Southwood
@ 2014-02-13 10:08 ` Ian Abbott
2014-02-14 11:52 ` Ian Abbott
2014-02-13 18:25 ` Hartley Sweeten
2014-02-14 1:02 ` [PATCH v2] " Chase Southwood
2 siblings, 1 reply; 8+ messages in thread
From: Ian Abbott @ 2014-02-13 10:08 UTC (permalink / raw)
To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel
On 2014-02-13 03:29, Chase Southwood wrote:
> In this if-else conditional statement, if (chan < 16), but
> (data[0] == INSN_CONFIG_DIO_QUERY), the function does not return early,
> but the else-branch does not get executed either. As a result, mask
> would be used uninitialized in the next line. What we want here is if
> (chan < 16) and (data[0] != INSN_CONFIG_DIO_QUERY), return an error, but
> in every other case, initialize mask and then proceed. Found by a static
> checker.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
> drivers/staging/comedi/drivers/addi_apci_3xxx.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> index ceadf8e..04c5153 100644
> --- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> +++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> @@ -688,13 +688,11 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
> * Port 1 (channels 8-15) are always outputs
> * Port 2 (channels 16-23) are programmable i/o
> */
> - if (chan < 16) {
> - if (data[0] != INSN_CONFIG_DIO_QUERY)
> - return -EINVAL;
> - } else {
> - /* changing any channel in port 2 changes the entire port */
> - mask = 0xff0000;
> - }
> + if ((chan < 16) && (data[0] != INSN_CONFIG_DIO_QUERY))
> + return -EINVAL;
> +
> + /* changing any channel in port 2 changes the entire port */
> + mask = 0xff0000;
>
> ret = comedi_dio_insn_config(dev, s, insn, data, mask);
> if (ret)
Seems correct.
Acked-by: Ian Abbott <abbotti@mev.co.uk>
--
-=( 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] 8+ messages in thread* Re: [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
2014-02-13 10:08 ` Ian Abbott
@ 2014-02-14 11:52 ` Ian Abbott
0 siblings, 0 replies; 8+ messages in thread
From: Ian Abbott @ 2014-02-14 11:52 UTC (permalink / raw)
To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel
On 2014-02-13 10:08, Ian Abbott wrote:
> On 2014-02-13 03:29, Chase Southwood wrote:
>> In this if-else conditional statement, if (chan < 16), but
>> (data[0] == INSN_CONFIG_DIO_QUERY), the function does not return early,
>> but the else-branch does not get executed either. As a result, mask
>> would be used uninitialized in the next line. What we want here is if
>> (chan < 16) and (data[0] != INSN_CONFIG_DIO_QUERY), return an error, but
>> in every other case, initialize mask and then proceed. Found by a static
>> checker.
>>
>> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
>> ---
>> drivers/staging/comedi/drivers/addi_apci_3xxx.c | 12 +++++-------
>> 1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>> b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>> index ceadf8e..04c5153 100644
>> --- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>> +++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>> @@ -688,13 +688,11 @@ static int apci3xxx_dio_insn_config(struct
>> comedi_device *dev,
>> * Port 1 (channels 8-15) are always outputs
>> * Port 2 (channels 16-23) are programmable i/o
>> */
>> - if (chan < 16) {
>> - if (data[0] != INSN_CONFIG_DIO_QUERY)
>> - return -EINVAL;
>> - } else {
>> - /* changing any channel in port 2 changes the entire port */
>> - mask = 0xff0000;
>> - }
>> + if ((chan < 16) && (data[0] != INSN_CONFIG_DIO_QUERY))
>> + return -EINVAL;
>> +
>> + /* changing any channel in port 2 changes the entire port */
>> + mask = 0xff0000;
>>
>> ret = comedi_dio_insn_config(dev, s, insn, data, mask);
>> if (ret)
>
> Seems correct.
>
> Acked-by: Ian Abbott <abbotti@mev.co.uk>
Okay, as Hartley pointed out, it isn't correct, so I withdraw my
"Acked-by".
--
-=( 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] 8+ messages in thread
* RE: [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
2014-02-13 3:29 [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c Chase Southwood
2014-02-13 10:08 ` Ian Abbott
@ 2014-02-13 18:25 ` Hartley Sweeten
2014-02-14 11:49 ` Ian Abbott
2014-02-14 1:02 ` [PATCH v2] " Chase Southwood
2 siblings, 1 reply; 8+ messages in thread
From: Hartley Sweeten @ 2014-02-13 18:25 UTC (permalink / raw)
To: Chase Southwood, gregkh; +Cc: abbotti, devel, linux-kernel
On Wednesday, February 12, 2014 8:29 PM, Chase Southwood wrote:
> In this if-else conditional statement, if (chan < 16), but
> (data[0] == INSN_CONFIG_DIO_QUERY), the function does not return early,
> but the else-branch does not get executed either. As a result, mask
> would be used uninitialized in the next line. What we want here is if
> (chan < 16) and (data[0] != INSN_CONFIG_DIO_QUERY), return an error, but
> in every other case, initialize mask and then proceed. Found by a static
> checker.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
> drivers/staging/comedi/drivers/addi_apci_3xxx.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> index ceadf8e..04c5153 100644
> --- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> +++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> @@ -688,13 +688,11 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
> * Port 1 (channels 8-15) are always outputs
> * Port 2 (channels 16-23) are programmable i/o
> */
> - if (chan < 16) {
> - if (data[0] != INSN_CONFIG_DIO_QUERY)
> - return -EINVAL;
> - } else {
> - /* changing any channel in port 2 changes the entire port */
> - mask = 0xff0000;
> - }
> + if ((chan < 16) && (data[0] != INSN_CONFIG_DIO_QUERY))
> + return -EINVAL;
> +
> + /* changing any channel in port 2 changes the entire port */
> + mask = 0xff0000;
>
> ret = comedi_dio_insn_config(dev, s, insn, data, mask);
> if (ret)
The uninitialized mask when chan < 16 is an issue. But your patch is not quite correct.
The original code was intending to limit the valid instructions for channels < 16 to only
INSN_CONFIG_DIO_QUERY. These channels have fixed directions: 0-7 (port 0) are
always inputs and 8-15 (port 1) are always outputs. Channels 16-23 (port 2) have
programmable direction but changing any channel effects the entire port, that's
what the 0xff0000 mask is for.
Changing the mask to 0xff0000 for any chanspec will result in the INSN_CONFIG_DIO_QUERY
instruction returning the direction of port 2 regardless of what the chanspec is.
The "right" fix would be:
1) Default the mask to 0 so that comedi_dio_insn_config() will use a chan_mask
based on the chanspec for the INSN_CONFIG_DIO_QUERY instruction.
2) Ignore all instructions except INSN_CONFIG_DIO_QUERY when the chan < 16.
3) Modify the mask for chan >= 16 when the instruction is not INSN_CONFIG_DIO_QUERY
so that the INSN_CONFIG_DIO_{INPUT,OUTPUT} instructions update the entire
port.
Something like the following patch is more correct. The comments are added
just for clarity.
Regards,
Hartley
diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
index 88d14a9..6bc8b26 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
@@ -706,7 +706,7 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
unsigned int *data)
{
unsigned int chan = CR_CHAN(insn->chanspec);
- unsigned int mask;
+ unsigned int mask = 0; /* use chan_mask in comedi_dio_insn_config() */
int ret;
/*
@@ -714,11 +714,16 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
* Port 1 (channels 8-15) are always outputs
* Port 2 (channels 16-23) are programmable i/o
*/
- if (chan < 16) {
- if (data[0] != INSN_CONFIG_DIO_QUERY)
+ if (data[0] != INSN_CONFIG_DIO_QUERY) {
+ /* ignore all other instructions to ports 0 and 1 */
+ if (chan < 16)
return -EINVAL;
- } else {
- /* changing any channel in port 2 changes the entire port */
+
+ /*
+ * Changing any channel in port 2 changes the entire port.
+ * Pass a custom mask to comedi_dio_insn_config() so all
+ * the io_bits are modified for port 2.
+ */
mask = 0xff0000;
}
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
2014-02-13 18:25 ` Hartley Sweeten
@ 2014-02-14 11:49 ` Ian Abbott
2014-02-14 17:54 ` Hartley Sweeten
0 siblings, 1 reply; 8+ messages in thread
From: Ian Abbott @ 2014-02-14 11:49 UTC (permalink / raw)
To: Hartley Sweeten, Chase Southwood, gregkh; +Cc: devel, linux-kernel
On 2014-02-13 18:25, Hartley Sweeten wrote:
> On Wednesday, February 12, 2014 8:29 PM, Chase Southwood wrote:
>> In this if-else conditional statement, if (chan < 16), but
>> (data[0] == INSN_CONFIG_DIO_QUERY), the function does not return early,
>> but the else-branch does not get executed either. As a result, mask
>> would be used uninitialized in the next line. What we want here is if
>> (chan < 16) and (data[0] != INSN_CONFIG_DIO_QUERY), return an error, but
>> in every other case, initialize mask and then proceed. Found by a static
>> checker.
>>
>> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
>> ---
>> drivers/staging/comedi/drivers/addi_apci_3xxx.c | 12 +++++-------
>> 1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>> index ceadf8e..04c5153 100644
>> --- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>> +++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>> @@ -688,13 +688,11 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
>> * Port 1 (channels 8-15) are always outputs
>> * Port 2 (channels 16-23) are programmable i/o
>> */
>> - if (chan < 16) {
>> - if (data[0] != INSN_CONFIG_DIO_QUERY)
>> - return -EINVAL;
>> - } else {
>> - /* changing any channel in port 2 changes the entire port */
>> - mask = 0xff0000;
>> - }
>> + if ((chan < 16) && (data[0] != INSN_CONFIG_DIO_QUERY))
>> + return -EINVAL;
>> +
>> + /* changing any channel in port 2 changes the entire port */
>> + mask = 0xff0000;
>>
>> ret = comedi_dio_insn_config(dev, s, insn, data, mask);
>> if (ret)
>
> The uninitialized mask when chan < 16 is an issue. But your patch is not quite correct.
>
> The original code was intending to limit the valid instructions for channels < 16 to only
> INSN_CONFIG_DIO_QUERY. These channels have fixed directions: 0-7 (port 0) are
> always inputs and 8-15 (port 1) are always outputs. Channels 16-23 (port 2) have
> programmable direction but changing any channel effects the entire port, that's
> what the 0xff0000 mask is for.
>
> Changing the mask to 0xff0000 for any chanspec will result in the INSN_CONFIG_DIO_QUERY
> instruction returning the direction of port 2 regardless of what the chanspec is.
>
> The "right" fix would be:
> 1) Default the mask to 0 so that comedi_dio_insn_config() will use a chan_mask
> based on the chanspec for the INSN_CONFIG_DIO_QUERY instruction.
> 2) Ignore all instructions except INSN_CONFIG_DIO_QUERY when the chan < 16.
> 3) Modify the mask for chan >= 16 when the instruction is not INSN_CONFIG_DIO_QUERY
> so that the INSN_CONFIG_DIO_{INPUT,OUTPUT} instructions update the entire
> port.
Agreed, but another question is why does comedi_dio_insn_config() in
drivers.c need to look at the supplied mask at all for
INSN_CONFIG_DIO_QUERY?
--
-=( 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] 8+ messages in thread* RE: [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
2014-02-14 11:49 ` Ian Abbott
@ 2014-02-14 17:54 ` Hartley Sweeten
0 siblings, 0 replies; 8+ messages in thread
From: Hartley Sweeten @ 2014-02-14 17:54 UTC (permalink / raw)
To: Ian Abbott, Chase Southwood, gregkh; +Cc: devel, linux-kernel
On Friday, February 14, 2014 4:50 AM, Ian Abbott wrote:
> On 2014-02-13 18:25, Hartley Sweeten wrote:
>> On Wednesday, February 12, 2014 8:29 PM, Chase Southwood wrote:
>>> In this if-else conditional statement, if (chan < 16), but
>>> (data[0] == INSN_CONFIG_DIO_QUERY), the function does not return early,
>>> but the else-branch does not get executed either. As a result, mask
>>> would be used uninitialized in the next line. What we want here is if
>>> (chan < 16) and (data[0] != INSN_CONFIG_DIO_QUERY), return an error, but
>>> in every other case, initialize mask and then proceed. Found by a static
>>> checker.
>>>
>>> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
>>> ---
>>> drivers/staging/comedi/drivers/addi_apci_3xxx.c | 12 +++++-------
>>> 1 file changed, 5 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>>> index ceadf8e..04c5153 100644
>>> --- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>>> +++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
>>> @@ -688,13 +688,11 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
>>> * Port 1 (channels 8-15) are always outputs
>>> * Port 2 (channels 16-23) are programmable i/o
>>> */
>>> - if (chan < 16) {
>>> - if (data[0] != INSN_CONFIG_DIO_QUERY)
>>> - return -EINVAL;
>>> - } else {
>>> - /* changing any channel in port 2 changes the entire port */
>>> - mask = 0xff0000;
>>> - }
>>> + if ((chan < 16) && (data[0] != INSN_CONFIG_DIO_QUERY))
>>> + return -EINVAL;
>>> +
>>> + /* changing any channel in port 2 changes the entire port */
>>> + mask = 0xff0000;
>>>
>>> ret = comedi_dio_insn_config(dev, s, insn, data, mask);
>>> if (ret)
>>
>> The uninitialized mask when chan < 16 is an issue. But your patch is not quite correct.
>>
>> The original code was intending to limit the valid instructions for channels < 16 to only
>> INSN_CONFIG_DIO_QUERY. These channels have fixed directions: 0-7 (port 0) are
>> always inputs and 8-15 (port 1) are always outputs. Channels 16-23 (port 2) have
>> programmable direction but changing any channel effects the entire port, that's
>> what the 0xff0000 mask is for.
>>
>> Changing the mask to 0xff0000 for any chanspec will result in the INSN_CONFIG_DIO_QUERY
>> instruction returning the direction of port 2 regardless of what the chanspec is.
>>
>> The "right" fix would be:
>> 1) Default the mask to 0 so that comedi_dio_insn_config() will use a chan_mask
>> based on the chanspec for the INSN_CONFIG_DIO_QUERY instruction.
>> 2) Ignore all instructions except INSN_CONFIG_DIO_QUERY when the chan < 16.
>> 3) Modify the mask for chan >= 16 when the instruction is not INSN_CONFIG_DIO_QUERY
>> so that the INSN_CONFIG_DIO_{INPUT,OUTPUT} instructions update the entire
>> port.
>
> Agreed, but another question is why does comedi_dio_insn_config() in
> drivers.c need to look at the supplied mask at all for
> INSN_CONFIG_DIO_QUERY?
It doesn't, that's why the mask is set to a default of 0 (1 above) and only modified
(3 above) when the instruction is not INSN_CONFIG_DIO_QUERY and chan >= 16.
With a mask of 0 comedi_dio_insn_config() will properly figure out the mask based
on the chanspec.
Regards,
Hartley
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
2014-02-13 3:29 [PATCH] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c Chase Southwood
2014-02-13 10:08 ` Ian Abbott
2014-02-13 18:25 ` Hartley Sweeten
@ 2014-02-14 1:02 ` Chase Southwood
2014-02-14 11:54 ` Ian Abbott
2 siblings, 1 reply; 8+ messages in thread
From: Chase Southwood @ 2014-02-14 1:02 UTC (permalink / raw)
To: gregkh; +Cc: abbotti, hsweeten, devel, linux-kernel, Chase Southwood
In this conditional statement, if (chan < 16), but the instruction passed
in data[0] is INSN_CONFIG_DIO_QUERY, the function does not return early,
but the else-branch does not get executed either. As a result, mask
would be used uninitialized in the next line. We want
comedi_dio_insn_config() to use a chan_mask based on the chanspec for the
INSN_CONFIG_DIO_QUERY instruction, so mask should be initialized to 0.
Then, if instead the instruction is INSN_CONFIG_DIO_{INPUT,OUTPUT}, we
return an error if (chan < 16) as these are invalid instructions for
ports 0 and 1, or update the mask otherwise, so all the io_bits are
modified for port 2. This ensures that mask is always initialized by the
time it is used.
Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---
2: Addressed all of the comments provided by Hartley regarding correct
structure of this patch.
Hopefully everything looks better!
drivers/staging/comedi/drivers/addi_apci_3xxx.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
index ceadf8e..1f3b668 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
@@ -680,7 +680,7 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
unsigned int *data)
{
unsigned int chan = CR_CHAN(insn->chanspec);
- unsigned int mask;
+ unsigned int mask = 0;
int ret;
/*
@@ -688,12 +688,13 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
* Port 1 (channels 8-15) are always outputs
* Port 2 (channels 16-23) are programmable i/o
*/
- if (chan < 16) {
- if (data[0] != INSN_CONFIG_DIO_QUERY)
+ if (data[0] != INSN_CONFIG_DIO_QUERY) {
+ /* ignore all other instructions for ports 0 and 1 */
+ if (chan < 16)
return -EINVAL;
- } else {
- /* changing any channel in port 2 changes the entire port */
- mask = 0xff0000;
+ else
+ /* changing any channel in port 2 changes the entire port */
+ mask = 0xff0000;
}
ret = comedi_dio_insn_config(dev, s, insn, data, mask);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] Staging: comedi: clean up conditional statement in addi_apci_3xxx.c
2014-02-14 1:02 ` [PATCH v2] " Chase Southwood
@ 2014-02-14 11:54 ` Ian Abbott
0 siblings, 0 replies; 8+ messages in thread
From: Ian Abbott @ 2014-02-14 11:54 UTC (permalink / raw)
To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel
On 2014-02-14 01:02, Chase Southwood wrote:
> In this conditional statement, if (chan < 16), but the instruction passed
> in data[0] is INSN_CONFIG_DIO_QUERY, the function does not return early,
> but the else-branch does not get executed either. As a result, mask
> would be used uninitialized in the next line. We want
> comedi_dio_insn_config() to use a chan_mask based on the chanspec for the
> INSN_CONFIG_DIO_QUERY instruction, so mask should be initialized to 0.
> Then, if instead the instruction is INSN_CONFIG_DIO_{INPUT,OUTPUT}, we
> return an error if (chan < 16) as these are invalid instructions for
> ports 0 and 1, or update the mask otherwise, so all the io_bits are
> modified for port 2. This ensures that mask is always initialized by the
> time it is used.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
>
> 2: Addressed all of the comments provided by Hartley regarding correct
> structure of this patch.
>
> Hopefully everything looks better!
>
> drivers/staging/comedi/drivers/addi_apci_3xxx.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> index ceadf8e..1f3b668 100644
> --- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> +++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
> @@ -680,7 +680,7 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
> unsigned int *data)
> {
> unsigned int chan = CR_CHAN(insn->chanspec);
> - unsigned int mask;
> + unsigned int mask = 0;
> int ret;
>
> /*
> @@ -688,12 +688,13 @@ static int apci3xxx_dio_insn_config(struct comedi_device *dev,
> * Port 1 (channels 8-15) are always outputs
> * Port 2 (channels 16-23) are programmable i/o
> */
> - if (chan < 16) {
> - if (data[0] != INSN_CONFIG_DIO_QUERY)
> + if (data[0] != INSN_CONFIG_DIO_QUERY) {
> + /* ignore all other instructions for ports 0 and 1 */
> + if (chan < 16)
> return -EINVAL;
> - } else {
> - /* changing any channel in port 2 changes the entire port */
> - mask = 0xff0000;
> + else
> + /* changing any channel in port 2 changes the entire port */
> + mask = 0xff0000;
> }
>
> ret = comedi_dio_insn_config(dev, s, insn, data, mask);
>
Looks okay this time. :)
Reviewed-by: Ian Abbott <abbotti@mev.co.uk>
--
-=( 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] 8+ messages in thread