Re: [libgpiod][RFC v2] core: implement v2.0 API

[Kent Gibson <warthog618@xxxxxxxxx>]

On Sat, May 29, 2021 at 08:19:32PM +0200, Bartosz Golaszewski wrote:
> On Sat, May 29, 2021 at 1:23 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >
> 

[snip]

> > > >
> > > > I would rename this to gpiod_line_config_set_output() and have it set
> > > > the direction (to GPIOD_LINE_CONFIG_DIRECTION_OUTPUT of course) as well
> > > > as the value.
> > > >
> > >
> > > I like it but how would that plug into the global direction setting?
> > >
> >
> > You mean the default flags?
> > You can work that out as part of the translation to uAPI.
> > It would be whichever applies to the most lines.
> > No need for the user to know or care.
> >
> 
> Eek this sounds vague. If that was part of documentation for this
> function - as a user I would be confused. Is there any problem with
> just implying output for any line for which a default value is set?
> 

What part of "No need for the user to know or care" is too vague?
The user does not need to know how libgpiod translates to uAPI - that
is your problem not theirs, so don't go there.

But for your benefit...

There is no global direction setting - the lines are treated
independently until the translation to uAPI during the request or
reconfigure call, at which point the most common flags setting can be
used as the global default.

The direction flag is only set for the lines specified in the set_output
call, so offset for the singular case, or offsets for the subset case.

> > > > And maybe add a gpiod_line_config_set_input()?
> > > >
> > >
> > > What about "as is" with this pattern? We agreed that input would be
> > > the default so we need some way to set "as is" too.
> > >
> >
> > I didn't suggest removing gpiod_line_config_set_direction(), so you would
> > use that.  The as-is case isn't sufficiently common to warrant its own
> > short form.
> >
> 
> 
> > I forgot to ask about where gpiod_line_info_get_name() and others that
> > return char * fit wrt that pattern.
> > So a string isn't a complex object?
> > Maybe they should be _peek_ as well?
> > Either way, it would be nice for their commentary to describe the lifetime
> > of the returned pointer.
> 
> With strings the common sense is to assume that returning char * means
> the string is dynamically allocated and must be freed by the caller,
> returning const char * means the string is stored in the container. I
> can't really recall seeing any other pattern in any sane C library. In
> any case - I will add a comment to every function that returns an
> object that needs lifetime management from the caller in v3.
> 

Sure, but don't assume common sense - document the behaviour - even if
the returned object doesn't require lifetime management by the caller.
They might assume they do - and free it for you.

And the point was that you still have some gets that return objects while
others return a reference - even after renaming some to peek, so the
pattern doesn't follow for the whole libgpiod API. Granted that is
nitpicking, but I would prefer internal self-consistency over following
what other libraries do.

Cheers,
Kent.