| Message ID | 20230315083456.27590-1-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [libgpiod] tools: gpiomon: fix setting event clock type | expand |
On Wed, Mar 15, 2023 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Fix an inverted logic bug when parsing event clock type in gpiomon. > > Fixes: 8ffb6489286f ("tools: line name focussed rework") > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Reported-by: Wes Tarro <wes.tarro@azuresummit.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > - if (strcmp(option, "hte") != 0) > + if (strcmp(option, "hte") == 0) I tend to code if (!strcmp(option, "hte")) but taste differs. Yours, Linus Walleij
On Wed, Mar 15, 2023 at 09:44:17AM +0100, Linus Walleij wrote: > On Wed, Mar 15, 2023 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > Fix an inverted logic bug when parsing event clock type in gpiomon. > > > > Fixes: 8ffb6489286f ("tools: line name focussed rework") > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Reported-by: Wes Tarro <wes.tarro@azuresummit.com> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > - if (strcmp(option, "hte") != 0) > > + if (strcmp(option, "hte") == 0) > > I tend to code if (!strcmp(option, "hte")) but taste differs. Pity we don't have match_string() API in a libc or somewhere in the user space. Allows to avoid such mistakes at once.
On Wed, Mar 15, 2023 at 09:34:56AM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Fix an inverted logic bug when parsing event clock type in gpiomon. Oh, oh. Good catch and fix! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Fixes: 8ffb6489286f ("tools: line name focussed rework") > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Reported-by: Wes Tarro <wes.tarro@azuresummit.com> > --- > tools/gpiomon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/gpiomon.c b/tools/gpiomon.c > index ec177df..c2684c2 100644 > --- a/tools/gpiomon.c > +++ b/tools/gpiomon.c > @@ -99,7 +99,7 @@ static int parse_event_clock_or_die(const char *option) > { > if (strcmp(option, "realtime") == 0) > return GPIOD_LINE_CLOCK_REALTIME; > - if (strcmp(option, "hte") != 0) > + if (strcmp(option, "hte") == 0) > return GPIOD_LINE_CLOCK_HTE; > if (strcmp(option, "monotonic") != 0) > die("invalid event clock: %s", option); > -- > 2.37.2 >
On Wed, 15 Mar 2023 at 15:04, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Wed, Mar 15, 2023 at 09:44:17AM +0100, Linus Walleij wrote: > > On Wed, Mar 15, 2023 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > Fix an inverted logic bug when parsing event clock type in gpiomon. > > > > > > Fixes: 8ffb6489286f ("tools: line name focussed rework") > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > Reported-by: Wes Tarro <wes.tarro@azuresummit.com> > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > > - if (strcmp(option, "hte") != 0) > > > + if (strcmp(option, "hte") == 0) > > > > I tend to code if (!strcmp(option, "hte")) but taste differs. > > Pity we don't have match_string() API in a libc or somewhere in the user space. > Allows to avoid such mistakes at once. > We have several instances of such string parsing in tools/ in libgpiod so feel free to add and use it. You can even port linux code verbatim - libgpiod is GPL after all. :) Bart
On Wed, Mar 15, 2023 at 9:35 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Fix an inverted logic bug when parsing event clock type in gpiomon. > > Fixes: 8ffb6489286f ("tools: line name focussed rework") > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Reported-by: Wes Tarro <wes.tarro@azuresummit.com> > --- > tools/gpiomon.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/gpiomon.c b/tools/gpiomon.c > index ec177df..c2684c2 100644 > --- a/tools/gpiomon.c > +++ b/tools/gpiomon.c > @@ -99,7 +99,7 @@ static int parse_event_clock_or_die(const char *option) > { > if (strcmp(option, "realtime") == 0) > return GPIOD_LINE_CLOCK_REALTIME; > - if (strcmp(option, "hte") != 0) > + if (strcmp(option, "hte") == 0) > return GPIOD_LINE_CLOCK_HTE; > if (strcmp(option, "monotonic") != 0) > die("invalid event clock: %s", option); > -- > 2.37.2 > Patch applied. Bart
diff --git a/tools/gpiomon.c b/tools/gpiomon.c index ec177df..c2684c2 100644 --- a/tools/gpiomon.c +++ b/tools/gpiomon.c @@ -99,7 +99,7 @@ static int parse_event_clock_or_die(const char *option) { if (strcmp(option, "realtime") == 0) return GPIOD_LINE_CLOCK_REALTIME; - if (strcmp(option, "hte") != 0) + if (strcmp(option, "hte") == 0) return GPIOD_LINE_CLOCK_HTE; if (strcmp(option, "monotonic") != 0) die("invalid event clock: %s", option);