Re: [libgpiod] gpiod_line_get_value_bulk may be broken?

[Kent Gibson <warthog618@gmail.com>]

On Thu, Aug 10, 2023 at 11:07:44PM +0100, andy pugh wrote:
> On Thu, 10 Aug 2023 at 01:46, Kent Gibson <warthog618@gmail.com> wrote:
> 
> > Check the sizes of the memory you are allocating.
> > e.g. this:
> >
> >             arr[c].chip = (struct gpiod_chip *)rtapi_kmalloc(sizeof(arr[c].chip), RTAPI_GFP_KERNEL);
> >
> > allocates the space for a pointer to a struct, not the struct itself.
> 
> Oh, that's embarrassing.
> 

Been there, done that.

> >             arr[c].chip = (struct gpiod_chip *)rtapi_kmalloc(sizeof(*arr[c].chip), RTAPI_GFP_KERNEL);
> 
> But that doesn't actually work:
> 
> "hal/drivers/hal_gpio.c: In function ‘build_chips_collection’:
> hal/drivers/hal_gpio.c:119:41: error: dereferencing pointer to
> incomplete type ‘struct gpiod_chip’
>       arr[c].chip = rtapi_kmalloc(sizeof(*arr[c].chip), RTAPI_GFP_KERNEL);
> "
> 
> The exact same structure compiles without error for the
> gpiod_line_bulk a few lines lower. I think that the difference is that
> gpiod.h includes the definition of struct gpiod_line_bulk, but only
> the prototype of struct gpiod_chip.
> (This is mainly based on googling Stack Overflow, so might be wrong)
> 

You need the full definition of gpiod_chip to determine its size, and it
seems gpiod_chip is opaque, even in v1.  Yay?
So you don't need to alloc for it at all - gpiod_line_find() already did
that. As per that function's comment, you still need to close the chip
eventually to prevent memory leaks.

Btw, you don't need to keep the chip open once you have the line requests.

> > There is no code movement at all, this is just telling the linker to
> > link libgpiod.a directly into your executable, rather than dynamically
> > linking against libgpiod.so.
> >
> > The specifics of how to do that depend on your build.
> 
> But surely the compiler and linker need either the V2 code, or the
> library object? How does it access these if we don't have them in our
> repository?

Your build needs libgpiod v2, and the linker will include the relevant
bits from that in your execuatable that you then package.
The dependency on libgpiod then is only build-time, not run-time.

> Even if we add it as a build-time dependency, it isn't currently
> available in the usual places?

If necessary, you install it from source for your build.

> I must be missing something? Or are you assuming that I am the only
> one compiling LinuxCNC?
> 

I am assuming you are building an execuable to package as a binary.
If you are building a dev package then you can't hide the libgpiod
dependency from the end user, so you have the choice of either using
what is readily available or forcing the end user to install libgpiod
from source themselves.

Cheers,
Kent.