* [PATCH] tools: iio: avoid returning error when channel does not have an offset
@ 2015-10-30 13:11 Ioana Ciornei
2015-10-31 10:13 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Ioana Ciornei @ 2015-10-30 13:11 UTC (permalink / raw)
To: linux-iio; +Cc: mranostay, Ioana Ciornei
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@gmail.com>
---
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)) {
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] tools: iio: avoid returning error when channel does not have an offset
2015-10-30 13:11 [PATCH] tools: iio: avoid returning error when channel does not have an offset Ioana Ciornei
@ 2015-10-31 10:13 ` Jonathan Cameron
2015-10-31 21:27 ` Ioana Ciornei
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2015-10-31 10:13 UTC (permalink / raw)
To: Ioana Ciornei, linux-iio; +Cc: mranostay
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@gmail.com>
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.
Perhaps we need to improve the documentation to make it clear that this is
the expected behaviour?
> ---
> 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)) {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools: iio: avoid returning error when channel does not have an offset
2015-10-31 10:13 ` Jonathan Cameron
@ 2015-10-31 21:27 ` Ioana Ciornei
2015-11-01 11:30 ` Ioana Ciornei
0 siblings, 1 reply; 6+ messages in thread
From: Ioana Ciornei @ 2015-10-31 21:27 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Matt Ranostaj
On Sat, Oct 31, 2015 at 12:13 PM, Jonathan Cameron <jic23@kernel.org> 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@gmail.com>
> 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.
>
> 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)) {
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools: iio: avoid returning error when channel does not have an offset
2015-10-31 21:27 ` Ioana Ciornei
@ 2015-11-01 11:30 ` Ioana Ciornei
2015-11-01 18:27 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Ioana Ciornei @ 2015-11-01 11:30 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Matt Ranostaj
On Sat, Oct 31, 2015 at 11:27 PM, Ioana Ciornei <ciorneiioana@gmail.com> wrote:
> On Sat, Oct 31, 2015 at 12:13 PM, Jonathan Cameron <jic23@kernel.org> 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@gmail.com>
>> 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..
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)) {
>>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools: iio: avoid returning error when channel does not have an offset
2015-11-01 11:30 ` Ioana Ciornei
@ 2015-11-01 18:27 ` Jonathan Cameron
2015-11-01 19:53 ` Ioana Ciornei
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2015-11-01 18:27 UTC (permalink / raw)
To: Ioana Ciornei; +Cc: linux-iio, Matt Ranostaj
On 01/11/15 11:30, Ioana Ciornei wrote:
> On Sat, Oct 31, 2015 at 11:27 PM, Ioana Ciornei <ciorneiioana@gmail.com> wrote:
>> On Sat, Oct 31, 2015 at 12:13 PM, Jonathan Cameron <jic23@kernel.org> 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@gmail.com>
>>> 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.
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)) {
>>>>
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] tools: iio: avoid returning error when channel does not have an offset
2015-11-01 18:27 ` Jonathan Cameron
@ 2015-11-01 19:53 ` Ioana Ciornei
0 siblings, 0 replies; 6+ messages in thread
From: Ioana Ciornei @ 2015-11-01 19:53 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Matt Ranostaj
On Sun, Nov 1, 2015 at 8:27 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 01/11/15 11:30, Ioana Ciornei wrote:
>> On Sat, Oct 31, 2015 at 11:27 PM, Ioana Ciornei <ciorneiioana@gmail.com> wrote:
>>> On Sat, Oct 31, 2015 at 12:13 PM, Jonathan Cameron <jic23@kernel.org> 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@gmail.com>
>>>> 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)) {
>>>>>
>>>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-01 19:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-30 13:11 [PATCH] tools: iio: avoid returning error when channel does not have an offset Ioana Ciornei
2015-10-31 10:13 ` Jonathan Cameron
2015-10-31 21:27 ` Ioana Ciornei
2015-11-01 11:30 ` Ioana Ciornei
2015-11-01 18:27 ` Jonathan Cameron
2015-11-01 19:53 ` Ioana Ciornei
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.