Re: libgpiod: rust bindings and bindgen issue with C enums

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 24, 2022 at 2:32 PM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Thu, Nov 24, 2022 at 04:15:01PM +0530, Viresh Kumar wrote:
> > On 23-11-22, 19:37, Bartosz Golaszewski wrote:
> > > Could you take a look at https://github.com/brgl/libgpiod-private?
> > > There's a branch called topic/further-libgpiod-v2-updates. Can you
> > > check out commit 5a4e08d546a8ec32757e6c9cc59d7a16939721ea and tell me
> > > how you'd make rust bindings work with it because I'm out of ideas
> > > (and my comfort zone)?
> >
> > https://github.com/vireshk/libgpiod brgl/fix
> >
> > For the benefit of others, I am pasting the entire diff of Rust changes required
> > to make the C library enums named.
> >
> > The part that can be improved, but I am not sure how, is the Error enum. Maybe
> > Miguel or Kent can help ?
> >
> > The problem is that the InvalidEnumValue Error needs to be generic, which makes
> > it:
> >
> > "
> > pub enum Error<E> {
> >     ...
> >     InvalidEnumValue(&'static str, E),
> > };
> >
> > pub type Result<T, E> = std::result::Result<T, Error<E>>;
> > "
> >
> > Where E can be i32 or u32. Currently I just cast it everywhere as i32 to make
> > it work.
> >
>
> I don't see this as a problem for generics.  Whether the enum is signed
> or unsigned doesn't need to affect the Error variant, much less the whole
> Error type.  The Error doesn't need to respresent the type of the source
> of the error, it needs to represent the type required to convey
> information to the user.
> Just accepting that the InvalidEnumValue variant expects i32, and casting
> from u32 if necessary, seems appropriate to me.  Unless there are some
> extreme values you are concerned about - but then you always switch it
> up to  i64 ;-).
>

In that case: Viresh: can I include your changes in my patch (giving
you credit as a co-author)?

Bart

> What is the problem that generics solve - that a subsequent bindgen or
> gpiod.h change might change signage on you?  If so then cast them all
> - even if the cast isn't necessary at present.
>
> Cheers,
> Kent.



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux