Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Fri, Jul 21, 2023 at 5:15 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Thu, Jul 20, 2023 at 04:47:47PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> >
> > Provide a wrapper around gpiod_line_request_get_chip_name() for Rust
> > bindings and add a test-case.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> > ---
> >  bindings/rust/libgpiod/src/line_request.rs   | 16 ++++++++++++++++
> >  bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs
> > index 1140aa9..737c06f 100644
> > --- a/bindings/rust/libgpiod/src/line_request.rs
> > +++ b/bindings/rust/libgpiod/src/line_request.rs
> > @@ -2,6 +2,7 @@
> >  // SPDX-FileCopyrightText: 2022 Linaro Ltd.
> >  // SPDX-FileCopyrightText: 2022 Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> >
> > +use std::ffi::CStr;
> >  use std::os::unix::prelude::AsRawFd;
> >  use std::time::Duration;
> >
> > @@ -25,6 +26,21 @@ impl Request {
> >          Ok(Self { request })
> >      }
> >
> > +    /// Get the name of the chip this request was made on.
> > +    pub fn chip_name(&self) -> Result<&str> {
> > +        // SAFETY: The `gpiod_line_request` is guaranteed to be live as long
> > +        // as `&self`
> > +        let name = unsafe { gpiod::gpiod_line_request_get_chip_name(self.request) };
> > +
> > +        // SAFETY: The string is guaranteed to be valid, non-null and immutable
> > +        // by the C API for the lifetime of the `gpiod_line_request`. The
> > +        // `gpiod_line_request` is living as long as `&self`. The string is
> > +        // returned read-only with a lifetime of `&self`.
> > +        unsafe { CStr::from_ptr(name) }
> > +            .to_str()
> > +            .map_err(Error::StringNotUtf8)
> > +    }
> > +
>
> I would drop the name temp var myself, but that is just a nit.
>

I would too but rust was making it very difficult with borrow semantics. :)

Bart

> Other than that the series looks good to me.
>
> Reviewed-by: Kent Gibson <warthog618@xxxxxxxxx>
>
> Cheers,
> Kent.