* [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.