Re: [libgpiod v2][PATCH V8 5/9] bindings: rust: Add libgpiod crate

[Kent Gibson <warthog618@xxxxxxxxx>]

On Mon, Oct 31, 2022 at 05:17:13PM +0530, Viresh Kumar wrote:
> Add rust wrapper crate, around the libpiod-sys crate added earlier, to
> provide a convenient interface for the users.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> ---
>  bindings/rust/Cargo.toml                     |   1 +
>  bindings/rust/libgpiod/Cargo.toml            |  20 +
>  bindings/rust/libgpiod/src/chip.rs           | 317 ++++++++++++
>  bindings/rust/libgpiod/src/edge_event.rs     | 128 +++++
>  bindings/rust/libgpiod/src/event_buffer.rs   | 102 ++++
>  bindings/rust/libgpiod/src/info_event.rs     |  69 +++
>  bindings/rust/libgpiod/src/lib.rs            | 478 +++++++++++++++++++
>  bindings/rust/libgpiod/src/line_config.rs    | 135 ++++++
>  bindings/rust/libgpiod/src/line_info.rs      | 162 +++++++
>  bindings/rust/libgpiod/src/line_request.rs   | 224 +++++++++
>  bindings/rust/libgpiod/src/line_settings.rs  | 297 ++++++++++++
>  bindings/rust/libgpiod/src/request_config.rs |  95 ++++
>  12 files changed, 2028 insertions(+)
>  create mode 100644 bindings/rust/libgpiod/Cargo.toml
>  create mode 100644 bindings/rust/libgpiod/src/chip.rs
>  create mode 100644 bindings/rust/libgpiod/src/edge_event.rs
>  create mode 100644 bindings/rust/libgpiod/src/event_buffer.rs
>  create mode 100644 bindings/rust/libgpiod/src/info_event.rs
>  create mode 100644 bindings/rust/libgpiod/src/lib.rs
>  create mode 100644 bindings/rust/libgpiod/src/line_config.rs
>  create mode 100644 bindings/rust/libgpiod/src/line_info.rs
>  create mode 100644 bindings/rust/libgpiod/src/line_request.rs
>  create mode 100644 bindings/rust/libgpiod/src/line_settings.rs
>  create mode 100644 bindings/rust/libgpiod/src/request_config.rs
> 
> diff --git a/bindings/rust/Cargo.toml b/bindings/rust/Cargo.toml
> index b9eea6b3a5ea..4fdf4e06ff90 100644
> --- a/bindings/rust/Cargo.toml
> +++ b/bindings/rust/Cargo.toml
> @@ -2,5 +2,6 @@
>  
>  members = [
>      "gpiosim",
> +    "libgpiod",
>      "libgpiod-sys"
>  ]
> diff --git a/bindings/rust/libgpiod/Cargo.toml b/bindings/rust/libgpiod/Cargo.toml
> new file mode 100644
> index 000000000000..ef52fdc198d7
> --- /dev/null
> +++ b/bindings/rust/libgpiod/Cargo.toml
> @@ -0,0 +1,20 @@
> +[package]
> +name = "libgpiod"
> +version = "0.1.0"
> +authors = ["Viresh Kumar <viresh.kumar@xxxxxxxxxx>"]
> +description = "libgpiod wrappers"
> +repository = "https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git";
> +categories = ["command-line-utilities", "os::linux-apis"]

The command line utilities being the examples?
That is a bit of a stretch.

How about "api-bindings", and maybe "hardware-support" and "embedded"?

> +rust-version = "1.56"
> +keywords = ["libgpiod", "gpio"]
> +license = "Apache-2.0 OR BSD-3-Clause"
> +edition = "2021"
> +
> +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
> +

That link is put there by cargo for YOU.
When you are satisfied with your keys you can delete it.

< --8<-- >

> +    /// Get the file descriptor associated with the chip.
> +    ///
> +    /// The returned file descriptor must not be closed by the caller, else other methods for the
> +    /// `struct Chip` may fail.
> +    pub fn fd(&self) -> Result<u32> {
> +        // SAFETY: `gpiod_chip` is guaranteed to be valid here.
> +        let fd = unsafe { gpiod::gpiod_chip_get_fd(self.ichip.chip) };
> +
> +        if fd < 0 {
> +            Err(Error::OperationFailed(
> +                OperationType::ChipGetFd,
> +                errno::errno(),
> +            ))
> +        } else {
> +            Ok(fd as u32)
> +        }
> +    }

Impl AsRawFd, as per Request.

< --8<-- >

> +/// Line info
> +///
> +/// Exposes functions for retrieving kernel information about both requested and
> +/// free lines.  Line info object contains an immutable snapshot of a line's status.
> +///
> +/// The line info contains all the publicly available information about a
> +/// line, which does not include the line value.  The line must be requested
> +/// to access the line value.
> +
> +#[derive(Debug, Eq, PartialEq)]
> +pub struct Info {
> +    info: *mut gpiod::gpiod_line_info,
> +    from_event: bool,
> +}

The "from_event" flag indicates if the info needs to be freed, or not,
when the Info is dropped, so call it something that indicates that, like
"contained".


Admittedly I'm only skimming this one compared to my v7 review,
but only minor knits that could always be picked up later.

Cheers,
Kent.