linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kent Gibson <warthog618@gmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Bartosz Golaszewski <brgl@bgdev.pl>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Subject: Re: libgpiod: rust bindings and bindgen issue with C enums
Date: Thu, 24 Nov 2022 21:32:25 +0800	[thread overview]
Message-ID: <Y39yackN2u7q2Fxs@sol> (raw)
In-Reply-To: <20221124104501.2bfllqmpfegdcs3m@vireshk-i7>

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 ;-).

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.

  reply	other threads:[~2022-11-24 13:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-22 15:37 libgpiod: rust bindings and bindgen issue with C enums Bartosz Golaszewski
2022-11-22 16:55 ` Miguel Ojeda
2022-11-22 19:08   ` Bartosz Golaszewski
2022-11-23 18:37     ` Bartosz Golaszewski
2022-11-24 10:45       ` Viresh Kumar
2022-11-24 13:32         ` Kent Gibson [this message]
2022-11-24 14:27           ` Bartosz Golaszewski
2022-11-25  5:49             ` Viresh Kumar
2022-11-24 14:46           ` Miguel Ojeda
2022-11-24 14:58             ` Kent Gibson
2022-11-24 16:18               ` Miguel Ojeda
2022-11-25  1:48                 ` Kent Gibson
2022-11-25  5:48           ` Viresh Kumar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y39yackN2u7q2Fxs@sol \
    --to=warthog618@gmail.com \
    --cc=brgl@bgdev.pl \
    --cc=linux-gpio@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).