lkml.org 
[lkml]   [2012]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] iio: inkern: add error case in iio_channel_get()
On 14/09/12 01:59, Kim, Milo wrote:
>> On 10/09/12 09:02, Kim, Milo wrote:
>>> The datasheet name is defined in the IIO driver.
>>> On the other hand, the adc_channel_label is configured in
>>> the platform machine side.
>>> If the datasheet name is not matched with any adc_channel_label,
>>> the iio_channel_get() should be returned as error for preventing
>>> using invalid IIO channel data.
>>>
>>> Moreover, this patch detects NULL pointer dereference problem at
>> early time.
>>> In general, the IIO driver just accesses to any member of the
>> iio_chan_spec
>>> in own xxx_read_raw() function.
>>> If the iio_chan_spec is invalid pointer, NULL dereference problem
>> may occur
>>> such like 'iio_chan_spec->channel' or 'iio_chan_spec->type'.
>>> If the iio_channel_get() gets failed in the IIO consumer,
>>> then no read_raw() operation proceeds.
>
>> I guess this is marginally cleaner, but either way it is up to the
>> calling driver to check whether the call succeeded.
>
> IMO, it would be better if iio_channel_get() gets failed
> when the consumer tries to get the channel rather than when using it.
>
> The channel spec is retrieved when the channel is requested, - iio_channel_get()
> So it's reasonable if error returns in case of invalid channel spcec.
>
> And I just found similar handling in iio_channel_get_all().
> But my patch needs releasing the memory as Lars-Peter noted.
Fine, I'm convinced. Certainly less weird doing it your way ;)
Please do fix up the get all case as well.
>
> Thanks !
>
> Best Regards,
> Milo
>
>>>
>>> Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
>>> ---
>>> drivers/iio/inkern.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>>> index 1faa240..a5caf6b 100644
>>> --- a/drivers/iio/inkern.c
>>> +++ b/drivers/iio/inkern.c
>>> @@ -136,11 +136,15 @@ struct iio_channel *iio_channel_get(const char
>> *name, const char *channel_name)
>>>
>>> channel->indio_dev = c->indio_dev;
>>>
>>> - if (c->map->adc_channel_label)
>>> + if (c->map->adc_channel_label) {
>>> channel->channel =
>>> iio_chan_spec_from_name(channel->indio_dev,
>>> c->map->adc_channel_label);
>>>
>>> + if (channel->channel == NULL)
>>> + return ERR_PTR(-ENODEV);
>>> + }
>>> +
>>> return channel;
>>> }
>>> EXPORT_SYMBOL_GPL(iio_channel_get);
>>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>



\
 
 \ /
  Last update: 2012-09-14 11:01    [W:0.083 / U:0.428 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site