Re: [libgpiod][PATCH] core: remove buggy flags sanitization from line-config

["J.A. Bezemer" <j.a.bezemer@xxxxxxxxxxxxxxxxxxxxx>]

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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxx>

For the record, this is fine with me too.

Thanks,
Anne Bezemer

Reviewed-by: Anne Bezemer <j.a.bezemer@xxxxxxxxxxxxxxxxxxxxx>

> > Reported-by: Anne Bezemer <j.a.bezemer@xxxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> > ---
> >  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
> >