linux-kernel - Re: [PATCH] comedi: Change error return code for if statement in the function,cb_pcimdas_ai

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Thu, 26 Feb 2015 11:09:56 +0000
From:	Ian Abbott <abbotti@....co.uk>
To:	Nicholas Krause <xerofoify@...il.com>,
	Hartley Sweeten <HartleyS@...ionengravers.com>
CC:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"ravi23ganiga@...il.com" <ravi23ganiga@...il.com>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] comedi: Change error return code for if statement in
 the function,cb_pcimdas_ai_rinsn

On 25/02/15 21:37, Nicholas Krause wrote:
>
>
> On February 25, 2015 1:03:14 PM EST, Hartley Sweeten <HartleyS@...ionengravers.com> wrote:
>> On Tuesday, February 24, 2015 9:13 PM, Nicholas Krause wrote:
>>> This changes us using the incorrect error,-ETIMEOUT when checking if
>>> the channel we are allocating to on the device structure pointer
>> passed
>>> to this function is greater then the maximum available channels for
>> this
>>> device to the correct error for a channel being out of range,-ECHRNG.
>>>
>>> Signed-off-by: Nicholas Krause <xerofoify@...il.com>
>>> ---
>>>   drivers/staging/comedi/drivers/cb_pcimdas.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/comedi/drivers/cb_pcimdas.c
>> b/drivers/staging/comedi/drivers/cb_pcimdas.c
>>> index 70dd2c9..d91a6f3 100644
>>> --- a/drivers/staging/comedi/drivers/cb_pcimdas.c
>>> +++ b/drivers/staging/comedi/drivers/cb_pcimdas.c
>>> @@ -121,7 +121,7 @@ static int cb_pcimdas_ai_rinsn(struct
>> comedi_device *dev,
>>>   		maxchans = s->n_chan;
>>>
>>>   	if (chan > (maxchans - 1))
>>> -		return -ETIMEDOUT;	/* *** Wrong error code. Fixme. */
>>> +		return -ECHRNG;
>>>
>>>   	/* configure for sw initiated read */
>>>   	d = inb(devpriv->BADR3 + 5);
>>
>> Hmm... This isn't quite right...
>>
>> The 16 single-ended / 8 differential analog input channels on this
>> board is
>> set with a switch on the PCB. The state of the switch should be read
>> when
>> the driver is attached and the subdevice initialized with the correct
>> number
>> of channels. The core will then validate the "chan" number before
>> calling
>> the (*insn_read) operation.
>>
>> Regards,
>> Hartley
> Hartley,
> If that is the case then why is the check  for max channels here.  I  can send in a v2 removing this check as it seems unneeded based on my understanding and your response to this patch.
> Nick
>

The patch is fine, but now superceded by Hartley's series of patches 
which has more extensive changes to the driver:

http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-February/065754.html

That set of patches assumes the switches remain in the same state 
through the lifetime of the device instance, which seems reasonable for 
a PCI card.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/