Re: [libgpiod][PATCH 15/16] bindings: rust: make mutators return &mut self

[Viresh Kumar <viresh.kumar@xxxxxxxxxx>]

On 16-01-23, 09:42, Bartosz Golaszewski wrote:
> On Mon, Jan 16, 2023 at 7:02 AM Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> >
> > On 13-01-23, 22:52, Bartosz Golaszewski wrote:
> > > diff --git a/bindings/rust/libgpiod/tests/line_request.rs b/bindings/rust/libgpiod/tests/line_request.rs
> > > index c3fc37b..561f4e8 100644
> > > --- a/bindings/rust/libgpiod/tests/line_request.rs
> > > +++ b/bindings/rust/libgpiod/tests/line_request.rs
> > > @@ -97,13 +97,11 @@ mod line_request {
> > >              config.lconfig_add_settings(&offsets);
> > >              config.request_lines().unwrap();
> > >
> > > -            let request = config.request();
> > > -
> >
> > Why remove this ? And open-code it ?
> >
> 
> Because I'm a Rust noob and couldn't figure out how to deal with the
> mutable/non-mutable borrow vomit that ensued when I kept the local
> variable. If you could improve upon this one, it would be great!

>From what I can understand, config.request() returns a mutable
reference and you don't need a mutable variable to keep it, since the
variable doesn't need to store another reference later. Just moving
back to original code should work here.

diff --git a/bindings/rust/libgpiod/tests/line_request.rs b/bindings/rust/libgpiod/tests/line_request.rs
index 561f4e80d189..5644292d486a 100644
--- a/bindings/rust/libgpiod/tests/line_request.rs
+++ b/bindings/rust/libgpiod/tests/line_request.rs
@@ -97,11 +97,13 @@ mod line_request {
             config.lconfig_add_settings(&offsets);
             config.request_lines().unwrap();

+            let request = config.request();
+
             // Single values read properly
-            assert_eq!(config.request().value(7).unwrap(), Value::Active);
+            assert_eq!(request.value(7).unwrap(), Value::Active);

             // Values read properly
-            let map = config.request().values().unwrap();
+            let map = request.values().unwrap();
             for i in 0..offsets.len() {
                 assert_eq!(
                     *map.get(offsets[i].into()).unwrap(),
@@ -113,7 +115,7 @@ mod line_request {
             }

             // Subset of values read properly
-            let map = config.request().values_subset(&[2, 0, 6]).unwrap();
+            let map = request.values_subset(&[2, 0, 6]).unwrap();
             assert_eq!(*map.get(2).unwrap(), Value::InActive);
             assert_eq!(*map.get(0).unwrap(), Value::InActive);
             assert_eq!(*map.get(6).unwrap(), Value::Active);
@@ -123,8 +125,8 @@ mod line_request {
             lsettings.set_active_low(true);
             let mut lconfig = line::Config::new().unwrap();
             lconfig.add_line_settings(&offsets, lsettings).unwrap();
-            config.request().reconfigure_lines(&lconfig).unwrap();
-            assert_eq!(config.request().value(7).unwrap(), Value::InActive);
+            request.reconfigure_lines(&lconfig).unwrap();
+            assert_eq!(request.value(7).unwrap(), Value::InActive);
         }

         #[test]

-- 
viresh