diff mbox series

[libgpiod,v3,2/3] bindings: rust: rename {event,settings}_clone to try_clone

Message ID 20231003-rust-line-info-soundness-v3-2-555ba21b4632@linaro.org
State Superseded
Headers show
Series bindings: rust: fix modeling of line_info lifetimes | expand

Commit Message

Erik Schilling Oct. 3, 2023, 9:39 a.m. UTC 
What is getting cloned is already clear from the type. This also aligns
a bit better with similar methods from the `std` crate [1].

[1] https://doc.rust-lang.org/std/index.html?search=try_clone

Link: https://lore.kernel.org/r/CVUKC1HXG1P8.13XIUCCXN95F0@ablu-work
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
---
 bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs | 2 +-
 bindings/rust/libgpiod/src/edge_event.rs                    | 3 ++-
 bindings/rust/libgpiod/src/line_settings.rs                 | 4 ++--
 bindings/rust/libgpiod/tests/line_request.rs                | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

Comments

Bartosz Golaszewski Oct. 4, 2023, 11:22 a.m. UTC | #1 
On Tue, Oct 3, 2023 at 11:40 AM Erik Schilling
<erik.schilling@linaro.org> wrote:
>
> What is getting cloned is already clear from the type. This also aligns
> a bit better with similar methods from the `std` crate [1].
>
> [1] https://doc.rust-lang.org/std/index.html?search=try_clone
>
> Link: https://lore.kernel.org/r/CVUKC1HXG1P8.13XIUCCXN95F0@ablu-work
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
> ---
>  bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs | 2 +-
>  bindings/rust/libgpiod/src/edge_event.rs                    | 3 ++-
>  bindings/rust/libgpiod/src/line_settings.rs                 | 4 ++--
>  bindings/rust/libgpiod/tests/line_request.rs                | 2 +-
>  4 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs b/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
> index ad90d7b..8dbb496 100644
> --- a/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
> +++ b/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
> @@ -34,7 +34,7 @@ fn main() -> libgpiod::Result<()> {
>          let event = events.next().unwrap()?;
>
>          // This will out live `event` and the next read_edge_events().
> -        let cloned_event = libgpiod::request::Event::event_clone(event)?;
> +        let cloned_event = libgpiod::request::Event::try_clone(event)?;
>
>          let events = request.read_edge_events(&mut buffer)?;
>          for event in events {
> diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs
> index 0c0cfbc..4c940ba 100644
> --- a/bindings/rust/libgpiod/src/edge_event.rs
> +++ b/bindings/rust/libgpiod/src/edge_event.rs
> @@ -25,7 +25,8 @@ use super::{
>  pub struct Event(*mut gpiod::gpiod_edge_event);
>
>  impl Event {
> -    pub fn event_clone(event: &Event) -> Result<Event> {
> +    /// Makes a copy of the event object.
> +    pub fn try_clone(event: &Event) -> Result<Event> {

Hi Erik,

This fails to apply on top of current master of libgpiod. Could you verify?

Bart

>          // SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
>          let event = unsafe { gpiod::gpiod_edge_event_copy(event.0) };
>          if event.is_null() {
> diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
> index f0b3e9c..41b27e2 100644
> --- a/bindings/rust/libgpiod/src/line_settings.rs
> +++ b/bindings/rust/libgpiod/src/line_settings.rs
> @@ -52,8 +52,8 @@ impl Settings {
>          unsafe { gpiod::gpiod_line_settings_reset(self.settings) }
>      }
>
> -    /// Makes copy of the settings object.
> -    pub fn settings_clone(&self) -> Result<Self> {
> +    /// Makes a copy of the settings object.
> +    pub fn try_clone(&self) -> Result<Self> {
>          // SAFETY: `gpiod_line_settings` is guaranteed to be valid here.
>          let settings = unsafe { gpiod::gpiod_line_settings_copy(self.settings) };
>          if settings.is_null() {
> diff --git a/bindings/rust/libgpiod/tests/line_request.rs b/bindings/rust/libgpiod/tests/line_request.rs
> index da22bea..e0ae200 100644
> --- a/bindings/rust/libgpiod/tests/line_request.rs
> +++ b/bindings/rust/libgpiod/tests/line_request.rs
> @@ -272,7 +272,7 @@ mod line_request {
>              for offset in offsets {
>                  lsettings.set_debounce_period(Duration::from_millis((100 + offset).into()));
>                  lconfig
> -                    .add_line_settings(&[offset as Offset], lsettings.settings_clone().unwrap())
> +                    .add_line_settings(&[offset as Offset], lsettings.try_clone().unwrap())
>                      .unwrap();
>              }
>
>
> --
> 2.41.0
>
Erik Schilling Oct. 4, 2023, 1:01 p.m. UTC | #2 
On Wed Oct 4, 2023 at 1:22 PM CEST, Bartosz Golaszewski wrote:
> On Tue, Oct 3, 2023 at 11:40 AM Erik Schilling
> <erik.schilling@linaro.org> wrote:
> >
> > What is getting cloned is already clear from the type. This also aligns
> > a bit better with similar methods from the `std` crate [1].
> >
> > [1] https://doc.rust-lang.org/std/index.html?search=try_clone
> >
> > Link: https://lore.kernel.org/r/CVUKC1HXG1P8.13XIUCCXN95F0@ablu-work
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
> > ---
> >  bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs | 2 +-
> >  bindings/rust/libgpiod/src/edge_event.rs                    | 3 ++-
> >  bindings/rust/libgpiod/src/line_settings.rs                 | 4 ++--
> >  bindings/rust/libgpiod/tests/line_request.rs                | 2 +-
> >  4 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs b/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
> > index ad90d7b..8dbb496 100644
> > --- a/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
> > +++ b/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
> > @@ -34,7 +34,7 @@ fn main() -> libgpiod::Result<()> {
> >          let event = events.next().unwrap()?;
> >
> >          // This will out live `event` and the next read_edge_events().
> > -        let cloned_event = libgpiod::request::Event::event_clone(event)?;
> > +        let cloned_event = libgpiod::request::Event::try_clone(event)?;
> >
> >          let events = request.read_edge_events(&mut buffer)?;
> >          for event in events {
> > diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs
> > index 0c0cfbc..4c940ba 100644
> > --- a/bindings/rust/libgpiod/src/edge_event.rs
> > +++ b/bindings/rust/libgpiod/src/edge_event.rs
> > @@ -25,7 +25,8 @@ use super::{
> >  pub struct Event(*mut gpiod::gpiod_edge_event);
> >
> >  impl Event {
> > -    pub fn event_clone(event: &Event) -> Result<Event> {
> > +    /// Makes a copy of the event object.
> > +    pub fn try_clone(event: &Event) -> Result<Event> {
>
> Hi Erik,
>
> This fails to apply on top of current master of libgpiod. Could you verify?

Hm. Not sure what went wrong. It rebased and tests cleanly for me.
Resent them.

- Erik

>
> Bart
>
> >          // SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
> >          let event = unsafe { gpiod::gpiod_edge_event_copy(event.0) };
> >          if event.is_null() {
> > diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
> > index f0b3e9c..41b27e2 100644
> > --- a/bindings/rust/libgpiod/src/line_settings.rs
> > +++ b/bindings/rust/libgpiod/src/line_settings.rs
> > @@ -52,8 +52,8 @@ impl Settings {
> >          unsafe { gpiod::gpiod_line_settings_reset(self.settings) }
> >      }
> >
> > -    /// Makes copy of the settings object.
> > -    pub fn settings_clone(&self) -> Result<Self> {
> > +    /// Makes a copy of the settings object.
> > +    pub fn try_clone(&self) -> Result<Self> {
> >          // SAFETY: `gpiod_line_settings` is guaranteed to be valid here.
> >          let settings = unsafe { gpiod::gpiod_line_settings_copy(self.settings) };
> >          if settings.is_null() {
> > diff --git a/bindings/rust/libgpiod/tests/line_request.rs b/bindings/rust/libgpiod/tests/line_request.rs
> > index da22bea..e0ae200 100644
> > --- a/bindings/rust/libgpiod/tests/line_request.rs
> > +++ b/bindings/rust/libgpiod/tests/line_request.rs
> > @@ -272,7 +272,7 @@ mod line_request {
> >              for offset in offsets {
> >                  lsettings.set_debounce_period(Duration::from_millis((100 + offset).into()));
> >                  lconfig
> > -                    .add_line_settings(&[offset as Offset], lsettings.settings_clone().unwrap())
> > +                    .add_line_settings(&[offset as Offset], lsettings.try_clone().unwrap())
> >                      .unwrap();
> >              }
> >
> >
> > --
> > 2.41.0
> >
diff mbox series

Patch

diff --git a/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs b/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
index ad90d7b..8dbb496 100644
--- a/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
+++ b/bindings/rust/libgpiod/examples/buffered_event_lifetimes.rs
@@ -34,7 +34,7 @@  fn main() -> libgpiod::Result<()> {
         let event = events.next().unwrap()?;
 
         // This will out live `event` and the next read_edge_events().
-        let cloned_event = libgpiod::request::Event::event_clone(event)?;
+        let cloned_event = libgpiod::request::Event::try_clone(event)?;
 
         let events = request.read_edge_events(&mut buffer)?;
         for event in events {
diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs
index 0c0cfbc..4c940ba 100644
--- a/bindings/rust/libgpiod/src/edge_event.rs
+++ b/bindings/rust/libgpiod/src/edge_event.rs
@@ -25,7 +25,8 @@  use super::{
 pub struct Event(*mut gpiod::gpiod_edge_event);
 
 impl Event {
-    pub fn event_clone(event: &Event) -> Result<Event> {
+    /// Makes a copy of the event object.
+    pub fn try_clone(event: &Event) -> Result<Event> {
         // SAFETY: `gpiod_edge_event` is guaranteed to be valid here.
         let event = unsafe { gpiod::gpiod_edge_event_copy(event.0) };
         if event.is_null() {
diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
index f0b3e9c..41b27e2 100644
--- a/bindings/rust/libgpiod/src/line_settings.rs
+++ b/bindings/rust/libgpiod/src/line_settings.rs
@@ -52,8 +52,8 @@  impl Settings {
         unsafe { gpiod::gpiod_line_settings_reset(self.settings) }
     }
 
-    /// Makes copy of the settings object.
-    pub fn settings_clone(&self) -> Result<Self> {
+    /// Makes a copy of the settings object.
+    pub fn try_clone(&self) -> Result<Self> {
         // SAFETY: `gpiod_line_settings` is guaranteed to be valid here.
         let settings = unsafe { gpiod::gpiod_line_settings_copy(self.settings) };
         if settings.is_null() {
diff --git a/bindings/rust/libgpiod/tests/line_request.rs b/bindings/rust/libgpiod/tests/line_request.rs
index da22bea..e0ae200 100644
--- a/bindings/rust/libgpiod/tests/line_request.rs
+++ b/bindings/rust/libgpiod/tests/line_request.rs
@@ -272,7 +272,7 @@  mod line_request {
             for offset in offsets {
                 lsettings.set_debounce_period(Duration::from_millis((100 + offset).into()));
                 lconfig
-                    .add_line_settings(&[offset as Offset], lsettings.settings_clone().unwrap())
+                    .add_line_settings(&[offset as Offset], lsettings.try_clone().unwrap())
                     .unwrap();
             }