From: "Björn Roy Baron" <bjorn3_gh@protonmail.com>
To: Viresh Kumar <viresh.kumar@linaro.org>
Cc: "Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com>,
"Linus Walleij" <linus.walleij@linaro.org>,
"Bartosz Golaszewski" <brgl@bgdev.pl>,
"Vincent Guittot" <vincent.guittot@linaro.org>,
linux-gpio@vger.kernel.org, "Kent Gibson" <warthog618@gmail.com>,
"Alex Bennée" <alex.bennee@linaro.org>,
stratos-dev@op-lists.linaro.org,
"Gerard Ryan" <g.m0n3y.2503@gmail.com>,
"Wedson Almeida Filho" <wedsonaf@gmail.com>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>, y86-dev <y86-dev@protonmail.com>
Subject: Re: [PATCH V7 0/8] libgpiod: Add Rust bindings
Date: Fri, 21 Oct 2022 14:34:44 +0000 [thread overview]
Message-ID: <YsZTLXUUjsBT-SzPWsI4ppoDBT_lGn0yfhF2r-5wfqcYxxeWWcdL2yIUcMMBmMTB-TjVUZkSqpMsvWNKTcSwoun5wzT4bbPi-ijZXUt8HWM=@protonmail.com> (raw)
In-Reply-To: <20221021093911.vj5todjdfqptdy7d@vireshk-i7>
On Friday, October 21st, 2022 at 11:39, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi Björn,
>
> I have bounced (mutt's feature) the initial emails to your and other
> email ids that Miguel added. The patches should be in your inbox now.
Thanks! I receive the patches.
>
> On 20-10-22, 13:29, Björn Roy Baron wrote:
>
> > At
> > https://github.com/vireshk/libgpiod/blob/3e7fb99173856a3995360fc3fad51220c4b4e90e/bindings/rust/libgpiod/src/lib.rs#L469
> > and elsewhere you might want to use `CStr::from_ptr(version)`. This
> > does the `strlen` call for you and you can convert it to an `&str`
> > using `.to_str()`.
>
> > At
> > https://github.com/vireshk/libgpiod/blob/3e7fb99173856a3995360fc3fad51220c4b4e90e/bindings/rust/libgpiod/src/chip.rs#L171
> > you could use `CString` and use the `.as_ptr()` method to get a
> > null-terminated string. Not sure if it would be nicer that what you
> > currently have though.
>
>
> These two were nice. Thanks.
>
> > At
> > https://github.com/vireshk/libgpiod/blob/3e7fb99173856a3995360fc3fad51220c4b4e90e/bindings/rust/libgpiod/src/edge_event.rs#L46
> > the lifetimes are unclear. Is Event allowed to outlive the buffer?
> > Can you add a lifetime annotation like fn event_clone<'a>(event:
> > &Event<'a>) -> Result<Event<'a>> if it isn't allowed to outlive the
> > buffer or fn event_clone<'a, 'b>(event: &Event<'a>) ->
> > Result<Event<'b>> if it is allowed to outlive the buffer. I'm not
> > sure which of the two the lifetime elision rules cause the current
> > code to be equivalent of, but even if it is correct, explicitly
> > stating the lifetime here is clearer IMHO.
>
>
> An Event created using Event::new() can't outlive the buffer, though
> an Event created using Event::event_clone() can.
>
> I tried to play around it based on your suggestion and here is the
> diff, does it make sense ?
>
> diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs
> index b36c23601bb4..0d328ebb2b03 100644
> --- a/bindings/rust/libgpiod/src/edge_event.rs
> +++ b/bindings/rust/libgpiod/src/edge_event.rs
> @@ -33,7 +33,7 @@ pub struct Event<'b> {
>
>
> impl<'b> Event<'b> {
>
> /// Get an event stored in the buffer.
> - pub(crate) fn new(buffer: &'b Buffer, index: usize) -> Result<Self> {
>
> + pub(crate) fn new(buffer: &'b Buffer, index: usize) -> Result<Event<'b>> {
This looks good to me.
>
> // SAFETY: The `gpiod_edge_event` returned by libgpiod is guaranteed to live as long
> // as the `struct Event`.
> let event = unsafe {
> @@ -52,22 +52,6 @@ impl<'b> Event<'b> {
>
> })
> }
>
> - pub fn event_clone(event: &Event) -> Result<Self> {
>
> - // SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
> - let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) };
> - if event.is_null() {
> - return Err(Error::OperationFailed(
> - OperationType::EdgeEventCopy,
> - Errno::last(),
> - ));
> - }
> -
> - Ok(Self {
> - buffer: None,
> - event,
> - })
> - }
> -
> /// Get the event type.
> pub fn event_type(&self) -> Result<EdgeKind> {
>
> // SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
> @@ -105,6 +89,27 @@ impl<'b> Event<'b> {
>
> }
> }
>
> +impl<'e, 'b> Event<'e> {
>
> + pub fn event_clone(event: &Event<'b>) -> Result<Event<'e>>
>
> + where
> + 'e: 'b,
Using `Event<'b>` on both sides should work fine. `Event` is covariant in it's lifetime parameter, so `Event<'b>` can be turned into `Event<'e>` with `'e` being a shorter lifetime than `'b`. What you wrote here is not incorrect, so if you prefer keeping it this way that is fine with me.
> + {
> + // SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
> + let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) };
> + if event.is_null() {
> + return Err(Error::OperationFailed(
> + OperationType::EdgeEventCopy,
> + Errno::last(),
> + ));
> + }
> +
> + Ok(Self {
> + buffer: None,
> + event,
> + })
> + }
> +}
> +
>
> --
> viresh
Cheers,
Björn
next prev parent reply other threads:[~2022-10-21 14:35 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-14 10:47 [PATCH V7 0/8] libgpiod: Add Rust bindings Viresh Kumar
2022-10-14 10:47 ` [PATCH V7 1/8] libgpiod: Add libgpiod-sys rust crate Viresh Kumar
2022-10-17 12:59 ` Kent Gibson
2022-10-18 11:22 ` Viresh Kumar
2022-10-18 11:42 ` Miguel Ojeda
2022-10-18 11:49 ` Kent Gibson
2022-10-19 6:46 ` Viresh Kumar
2022-10-19 7:21 ` Kent Gibson
2022-10-19 11:22 ` Viresh Kumar
2022-10-19 12:01 ` Kent Gibson
2022-10-20 5:34 ` Viresh Kumar
2022-10-27 12:18 ` Bartosz Golaszewski
2022-10-14 10:47 ` [PATCH V7 2/8] libgpiod-sys: Add pre generated rust bindings Viresh Kumar
2022-10-14 10:47 ` [PATCH V7 3/8] libgpiod: Add rust wrapper crate Viresh Kumar
2022-10-17 12:59 ` Kent Gibson
2022-10-21 11:22 ` Viresh Kumar
2022-10-21 12:39 ` Kent Gibson
2022-10-27 12:09 ` Bartosz Golaszewski
2022-10-31 12:33 ` Kent Gibson
2022-10-31 12:45 ` Bartosz Golaszewski
2022-11-02 4:00 ` Viresh Kumar
2022-11-02 12:47 ` Bartosz Golaszewski
2022-11-02 13:08 ` Kent Gibson
2022-11-02 16:34 ` Bartosz Golaszewski
2022-11-03 0:38 ` Kent Gibson
2022-11-03 8:35 ` Bartosz Golaszewski
2022-11-03 12:29 ` Kent Gibson
2022-11-04 15:51 ` Bartosz Golaszewski
2022-10-18 3:27 ` Kent Gibson
2022-10-21 11:25 ` Viresh Kumar
2022-10-14 10:47 ` [PATCH V7 4/8] libgpiod: Add rust examples Viresh Kumar
2022-10-17 13:00 ` Kent Gibson
2022-10-14 10:47 ` [PATCH V7 5/8] libgpiod: Add gpiosim rust crate Viresh Kumar
2022-10-17 13:00 ` Kent Gibson
2022-10-21 9:56 ` Viresh Kumar
2022-10-14 10:47 ` [PATCH V7 6/8] gpiosim: Add pre generated rust bindings Viresh Kumar
2022-10-14 10:47 ` [PATCH V7 7/8] libgpiod: Add rust tests Viresh Kumar
2022-10-17 13:00 ` Kent Gibson
2022-10-17 14:30 ` Kent Gibson
2022-10-20 10:37 ` Viresh Kumar
2022-10-20 11:02 ` Kent Gibson
2022-10-20 12:40 ` Bartosz Golaszewski
2022-10-20 15:16 ` Miguel Ojeda
2022-10-14 10:47 ` [PATCH V7 8/8] libgpiod: Integrate building of rust bindings with make Viresh Kumar
2022-10-14 17:03 ` [PATCH V7 0/8] libgpiod: Add Rust bindings Miguel Ojeda
2022-10-20 13:29 ` Björn Roy Baron
2022-10-21 9:39 ` Viresh Kumar
2022-10-21 14:34 ` Björn Roy Baron [this message]
2022-10-25 6:42 ` Viresh Kumar
2022-10-29 17:46 ` Björn Roy Baron
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='YsZTLXUUjsBT-SzPWsI4ppoDBT_lGn0yfhF2r-5wfqcYxxeWWcdL2yIUcMMBmMTB-TjVUZkSqpMsvWNKTcSwoun5wzT4bbPi-ijZXUt8HWM=@protonmail.com' \
--to=bjorn3_gh@protonmail.com \
--cc=alex.bennee@linaro.org \
--cc=alex.gaynor@gmail.com \
--cc=boqun.feng@gmail.com \
--cc=brgl@bgdev.pl \
--cc=g.m0n3y.2503@gmail.com \
--cc=gary@garyguo.net \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=stratos-dev@op-lists.linaro.org \
--cc=vincent.guittot@linaro.org \
--cc=viresh.kumar@linaro.org \
--cc=warthog618@gmail.com \
--cc=wedsonaf@gmail.com \
--cc=y86-dev@protonmail.com \
/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).