| Message ID | 20240104135058.46703-1-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [libgpiod] core: remove buggy flags sanitization from line-config | expand |
On Thu, Jan 04, 2024 at 02:50:58PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We try to drop potentially set output flags from line config if edge > detection is enabled but we use the library enum instead of the one from > the uAPI. In any case, we should actually loudly complain if user tries > to use the output mode with edge-detection (like we do currently) so just > remove offending lines entirely. > I don't see any problem with that. It also explains why we didn't pick it up earlier - it behaves as expected and has no visible side effects, so this is just tidying up dead code. Reviewed-by: Kent Gibson <warthog618@gmail.com> > Reported-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > lib/line-config.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/lib/line-config.c b/lib/line-config.c > index 2749a2a..9302c1b 100644 > --- a/lib/line-config.c > +++ b/lib/line-config.c > @@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings) > case GPIOD_LINE_EDGE_FALLING: > flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | > GPIO_V2_LINE_FLAG_INPUT); > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > break; > case GPIOD_LINE_EDGE_RISING: > flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING | > GPIO_V2_LINE_FLAG_INPUT); > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > break; > case GPIOD_LINE_EDGE_BOTH: > flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | > GPIO_V2_LINE_FLAG_EDGE_RISING | > GPIO_V2_LINE_FLAG_INPUT); > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > break; > default: > break; > -- > 2.40.1 >
On Thu, 4 Jan 2024, Kent Gibson wrote: > On Thu, Jan 04, 2024 at 02:50:58PM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > We try to drop potentially set output flags from line config if edge > > detection is enabled but we use the library enum instead of the one from > > the uAPI. In any case, we should actually loudly complain if user tries > > to use the output mode with edge-detection (like we do currently) so just > > remove offending lines entirely. > > > > I don't see any problem with that. > It also explains why we didn't pick it up earlier - it behaves as > expected and has no visible side effects, so this is just tidying up > dead code. > > Reviewed-by: Kent Gibson <warthog618@gmail.com> For the record, this is fine with me too. Thanks, Anne Bezemer Reviewed-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl> > > Reported-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl> > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > --- > > lib/line-config.c | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/lib/line-config.c b/lib/line-config.c > > index 2749a2a..9302c1b 100644 > > --- a/lib/line-config.c > > +++ b/lib/line-config.c > > @@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings) > > case GPIOD_LINE_EDGE_FALLING: > > flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | > > GPIO_V2_LINE_FLAG_INPUT); > > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > > break; > > case GPIOD_LINE_EDGE_RISING: > > flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING | > > GPIO_V2_LINE_FLAG_INPUT); > > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > > break; > > case GPIOD_LINE_EDGE_BOTH: > > flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | > > GPIO_V2_LINE_FLAG_EDGE_RISING | > > GPIO_V2_LINE_FLAG_INPUT); > > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > > break; > > default: > > break; > > -- > > 2.40.1 > >
On Thu, Jan 4, 2024 at 2:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > We try to drop potentially set output flags from line config if edge > detection is enabled but we use the library enum instead of the one from > the uAPI. In any case, we should actually loudly complain if user tries > to use the output mode with edge-detection (like we do currently) so just > remove offending lines entirely. > > Reported-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > lib/line-config.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/lib/line-config.c b/lib/line-config.c > index 2749a2a..9302c1b 100644 > --- a/lib/line-config.c > +++ b/lib/line-config.c > @@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings) > case GPIOD_LINE_EDGE_FALLING: > flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | > GPIO_V2_LINE_FLAG_INPUT); > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > break; > case GPIOD_LINE_EDGE_RISING: > flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING | > GPIO_V2_LINE_FLAG_INPUT); > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > break; > case GPIOD_LINE_EDGE_BOTH: > flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | > GPIO_V2_LINE_FLAG_EDGE_RISING | > GPIO_V2_LINE_FLAG_INPUT); > - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; > break; > default: > break; > -- > 2.40.1 > Patch applied. Bart
diff --git a/lib/line-config.c b/lib/line-config.c index 2749a2a..9302c1b 100644 --- a/lib/line-config.c +++ b/lib/line-config.c @@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings) case GPIOD_LINE_EDGE_FALLING: flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | GPIO_V2_LINE_FLAG_INPUT); - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; break; case GPIOD_LINE_EDGE_RISING: flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING | GPIO_V2_LINE_FLAG_INPUT); - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; break; case GPIOD_LINE_EDGE_BOTH: flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING | GPIO_V2_LINE_FLAG_EDGE_RISING | GPIO_V2_LINE_FLAG_INPUT); - flags &= ~GPIOD_LINE_DIRECTION_OUTPUT; break; default: break;