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

[Kent Gibson <warthog618@gmail.com>]

On Fri, Jul 21, 2023 at 08:35:07PM +0200, Bartosz Golaszewski wrote:
> On Fri, Jul 21, 2023 at 5:15 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Thu, Jul 20, 2023 at 04:47:47PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > 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@linaro.org>
> > > ---
> > >  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@linaro.org>
> > >
> > > +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. :)
> 

Really?  What error are you getting?

This works for me:

    /// Get the name of the chip this request was made on.
    pub fn chip_name(&self) -> Result<&str> {
        // 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(gpiod::gpiod_line_request_get_chip_name(self.request)) }
            .to_str()
            .map_err(Error::StringNotUtf8)
    }

And the last sentence of the SAFETY comment looks redundant to me -
it is just repeating what the signature already says.
(otherwise the return would be something like Result<&'a mut String>)

Cheers,
Kent.