Re: [libgpiod][RFC] helper functions for basic use cases

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Wed, May 22, 2024 at 2:41 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Wed, May 22, 2024 at 12:59:39PM +0200, Bartosz Golaszewski wrote:
> > On Fri, May 17, 2024 at 2:37 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> > >
> > >
> > > Oh, I agree - that makes total sense.  The direction I was headed felt wrong,
> > > so I figured I must've misunderstood what you meant.
> > >
> > > I'll re-organise it into a separate unit.
> > >
> > > Does that unit have to act through the core API, or can we give it
> > > access to the internals?
> >
> > If we can avoid it accessing the internals, that would be awesome.
> > Unless you hit a road-block, please try to keep the core separate.
> >
>
> If you want the ext functions to store config then that would rule out the
> shortcut constructors returning a gpiod_line_request.
>

Actually this is why I initially proposed to have another object
wrapping gpiod_line_request. I don't really want to extend it unless
we make use of it in the core. We can always provide
gpiod_ext_state_get_request() or something. We could call this wrapper
gpiod_ext_state so that we can cram whatever persistence we will
require as the ext library grows (which it inevitably will).

> I was thinking that the ext could be a friend of core and get access to
> some things not generally accessible, in this case allowing ext to store
> the config inside the request.
>

I would rather it be a wrapper around libgpiod which simplifies
operating on the core API.

> Without that, the two options we have is to rebuild the config from line
> info, which you don't like, or wrapping the gpiod_line_request and have
> the wrapper provide an accessor to the contained gpiod_line_request if
> the user wants to use core API, which I'm not thrilled about.
> But that seems to be the only option.
>

If I were to rank the solutions here from best to worst (personal
opinion) I'd go:

1. Wrapper for extended state providing accessor to gpiod_line_request
and storing the config
2. Rebuilding the config from info
3. Storing the config in gpiod_line_request

> > > And where do you stand wrt the idea of adding a config pointer to struct
> > > gpiod_line_request itself?
> > >
> >
> > We'd need to make a deep copy, otherwise it could be destroyed and the
> > pointer would be left dangling, right?
> >
>
> No, as the config is constructed and added by ext - the user can't actually
> see it.  It would only be accessed indirectly via the ext functions.
> If using only the core API then it would always be NULL.
>

Ugh, I really don't like having a useless pointer in this struct. It's
not about memory - the 8 bytes are irrelevant - but the logic of it.
Making concessions in the very core of the library for what is
essentially an outer shell around it just doesn't sound right.

> Well unless we were to provide accessors to make it accessible to the
> user, and then which way to go would be open to debate.  There are other
> functions in libgpiod that pass ownership, so copying isn't the only
> option. I haven't put much thought into that though, as I wasn't planning
> to go there.
>

For now that doesn't seem required. At worst we can provide a function
to lookup individual per offset settings from the ext state object.
Settings can be copied alright.

Bart