On Sun, Nov 1, 2015 at 8:27 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On 01/11/15 11:30, Ioana Ciornei wrote:
>> On Sat, Oct 31, 2015 at 11:27 PM, Ioana Ciornei <ciorneiioana@xxxxxxxxx> wrote:
>>> On Sat, Oct 31, 2015 at 12:13 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>>>> On 30/10/15 13:11, Ioana Ciornei wrote:
>>>>> This patch avoids getting an error and aborting when a channel does not have a
>>>>> specific parameter like 'offset' and the file associated with it is not present.
>>>>> When none of the files from the device folder does not matches the desired channel's
>>>>> param function iioutils_get_param_float should return 0.
>>>>>
>>>>> This change is safe previous to calling the function the parameters are set to
>>>>> their defaults, 0 in case of 'offset'
>>>>>
>>>>> Signed-off-by: Ioana Ciornei <ciorneiioana@xxxxxxxxx>
>>>> This is rather fixing it in the wrong place. This function is specifically getting
>>>> the parameter. If it is not there, then the function should return that fact.
>>>> It's the caller that should know whether the parameter is optional.
>>>>
>>>> For example, build_channel_array explicitly catches the -ENOENT error and allows
>>>> for that case.
>>>>
>>
>> I took a look at the build_channel_array and the
>> iioutils_get_param_float and now I do not understand how the generic
>> buffer tool fails at line 495 (where the check is made) if there is no
>> offset in the channel.
>> If there is no file in the device folder that matches *_offset it the
>> iioutils_get_param_float call should return -ENOENT and it should pass
>> that line since the check permits it.
>>
>> Sorry, but now I'm confused..
> Me too! Anyhow, we are clearly both missing something here.
>
> Could you add a few debugging printf lines and chase it through to find
> out if for some reason, in this case, the code is not returning -ENOENT
> from that call? I wonder if there is some other error occurring
> during that functional call.
>
> It's incredibly common to not have an offset so this really ought to be
> tested code paths. I can obviously rig something up to test, but given
> you already have a setup that is triggering the issue makes sense for
> you to pursue it if you have time.
I will definitely pursue this as soon as possible.
Thanks for the feedback.
Ioana
>
> Jonathan
>
>>
>> Ioana
>>
>>>> Perhaps we need to improve the documentation to make it clear that this is
>>>> the expected behaviour?
>>>
>>> Ok, sure. I will deal with the documentation in a patch.
>>>
>>> Sorry for the late response.
>>>
>>> Ioana
>>>
>>>>
>>>>> ---
>>>>> not tested
>>>>>
>>>>> tools/iio/iio_utils.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c
>>>>> index 5eb6793..cbaf696 100644
>>>>> --- a/tools/iio/iio_utils.c
>>>>> +++ b/tools/iio/iio_utils.c
>>>>> @@ -241,7 +241,7 @@ int iioutils_get_param_float(float *output, const char *param_name,
>>>>> goto error_free_builtname_generic;
>>>>> }
>>>>>
>>>>> - ret = -ENOENT;
>>>>> + ret = 0;
>>>>> while (ent = readdir(dp), ent)
>>>>> if ((strcmp(builtname, ent->d_name) == 0) ||
>>>>> (strcmp(builtname_generic, ent->d_name) == 0)) {
>>>>>
>>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
