Linux kernel commit 8bd76b3d3f3a ("gpio: sim: lock up configfs that an
instantiated device depends on") uncovered an issue in Rust bindings
tests where the GPIO simulator device is not disabled before removing its
configfs entries. Implenent Drop for the Sim struct in order to disable
the simulator first before dropping its bank and device objects.
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Link: https://lore.kernel.org/r/20250203-fix-gpiosim-in-bindings-v1-3-d2c9e0f3a29d@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
This was complained about by clippy:
warning: empty line after doc comment
--> libgpiod/src/lib.rs:461:1
|
461 | / /// Various libgpiod-related functions.
462 | |
| |_
...
467 | pub fn is_gpiochip_device<P: AsRef<Path>>(path: &P) -> bool {
| ----------------------------------------------------------- the comment documents this function
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments
= help: if the empty line is unintentional remove it
help: if the documentation should include the empty line include it in the comment
|
462 | ///
|
The comment more seems to be of structural matter and should not be tied
to the function immediately below it. So lets turn it into a normal
non-doc comment.
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Link: https://lore.kernel.org/r/20250103-lifetime-fix-v2-2-63902dc8cae1@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Newer rustc versions warn on elided named lifetimes. In thise case, it
actually revealed a misleading function signature.
The function defined &self with 'a. But there is no dependency on the
Request reference that extends longer than this function call. Yet,
the code expresses such a dependency and abuses it to force the buffer
argument to match &self lifetime (and thus the return lifetime too!).
Instead we want to explicitly break up the tie to the &self and tie it
to the buffer instead.
The 'a on &self caused a 'a on the return. With that being removed we
would get an automatic '0. So we are now forced to add the 'a on the
return that also fixes the compiler warning.
Reported-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Co-authored-by: Kent Gibson <warthog618@gmail.com>
Link: https://lore.kernel.org/r/CAMRc=Me-QNmJ2L1K-gGzFtVZacsDiLsNUfh3QaWPdbVzyxUduA@mail.gmail.com/
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Link: https://lore.kernel.org/r/20250103-lifetime-fix-v2-1-63902dc8cae1@linaro.org
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Linux kernel commit b44039638741 ("gpiolib: cdev: Ignore reconfiguration
without direction") made the direction setting mandatory for line config
passed to the kernel when reconfiguring requested lines. Fix the Rust
test cases which don't do it and now fail due to the rest of the
settings being ignored.
Reviewed-by: Kent Gibson <warthog618@gmail.com>
Link: https://lore.kernel.org/r/20240708094858.85015-1-brgl@bgdev.pl
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
During 86860fb ("bindings: rust: libgpiod: release 0.2.2"), I forgot that
we also need a libgpiod-sys release in order to expose the new feature
flag to raise the minimum libgpiod version.
Changelog:
7552e5d (bindings: rust: expose v2.1 features as flag, 2023-11-06)
bc91656 (bindings: rust: add wrapper.h to EXTRA_DIST, 2023-11-03)
2e6ee87 (bindings: rust: mention the libgpiod crate from libgpiod-sys, 2023-07-03)
d04639d (bindings: rust: bump MSRV to 1.60, 2023-06-16)
ebfed6c (bindings: rust: document build without install, 2023-06-12)
bce8623 (bindings: rust: exclude Makefile.am from package, 2023-06-12)
caabf53 (bindings: rust: add missing license and copyright boilerplate, 2023-06-13)
Most changes only touch the build scripts or modify packaging details.
Bumping the MSRV and introducing a new feature does not require a major
bump.
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Minor update the exposes feature flag and bindings for v2.1.
Changelog:
==========
7552e5d (bindings: rust: expose v2.1 features as flag, 2023-11-06)
bc91656 (bindings: rust: add wrapper.h to EXTRA_DIST, 2023-11-03)
3b40a37 (bindings: rust: fix EXTRA_DIST for examples, 2023-11-03)
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We need to package wrapper.h in the release tarball so add it to
EXTRA_DIST.
Fixes: 1f80859530 ("bindings: rust: build against pkg-config info")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
One of the examples listed in EXTRA_DIST is missing the .rs suffix while
another lists a file that doesn't exist. Fix both issues.
Fixes: aaed0f2e62 ("bindings: rust: examples: replace tools examples with use case examples")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
While the release technically removes a function, that function was not
available in any released libgpiod version. Therefore, this only bumps
the minor.
This release fixes building with releases from the 2.x series of the C
lib.
Changelog:
=========
acebcf2 (bindings: rust: feature gate unreleased features, 2023-10-06)
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
`gpiod_line_request_get_chip_name()` is not released yet. Still, libgpiod-sys
will just happily generate bindings for it if it sees the definition in the
header file.
This guards the unreleased features behind an optional feature `vnext`.
To sketch the process of what happens once these features get into an
assumed "2.1" release:
libgpiod-sys will get updated with a `v2_1` feature. That feature would
then raise the minimum version that is attempted to query from pkg-
config. An identical feature will then be introduced on the `libgpiod`
crate and `vnext` guards will be changed to `v2_1` guards. The `vnext`
feature will then be updated to require the new `v2_1` feature.
Eventually, we will probably raise the minimum supported version for the
rust bindings and drop all the version gates before that.
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Since we had some (potentially-)breaking changes, we bump the major for
the next release of the crate.
Note:
I am using the term "major" as defined in the Rust SemVer compatibility
guide [1], where the first non-zero digit is considered as "major".
[1] https://doc.rust-lang.org/cargo/reference/semver.html
Changelog:
=========
(potentially-)breaking changes:
a29f3e6 (bindings: rust: rename {event,settings}_clone to try_clone, 2023-10-04)
b290348 (bindings: rust: fix soundness of line_info modeling, 2023-10-03)
d04639d (bindings: rust: bump MSRV to 1.60, 2023-06-16)
new functionality:
808d15e (bindings: rust: allow cloning line::InfoRef -> line::Info, 2023-10-03)
64aac85 (bindings: rust: mark all owning types as `Send`, 2023-09-28)
d12ce74 (bindings: rust: provide LineRequest::chip_name(), 2023-07-20)
53226d5 (bindings: rust: examples: add dedicated examples, 2023-06-14)
other changes:
0a570b6 (bindings: rust: drop unneeded Arc within Chip, 2023-09-27)
a97fe96 (bindings: rust: construct chip infos by reference, 2023-09-27)
27afa47 (bindings: rust: remove useless clone, 2023-09-28)
3f6e0bf (bindings: rust: add README.md for libgpiod crate, 2023-07-03)
4b8357b (bindings: rust: clippy: silence false-positive on iterator, 2023-06-30)
39189f0 (bindings: rust: clippy: drop unneeded conversions, 2023-06-30)
46115fd (bindings: rust: clippy: silence false-positives on casts, 2023-06-30)
901104e (bindings: rust: clippy: drop unnecessary casts, 2023-06-30)
46ecbe0 (rust: examples: file comment consistency, 2023-06-24)
aaed0f2 (bindings: rust: examples: replace tools examples with use case examples, 2023-06-23)
b37bd9e (bindings: rust: examples: consistency cleanup, 2023-06-23)
06c8ad9 (bindings: rust: package new examples in the distro tarball, 2023-06-15)
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
While attention was provided to prevent freeing in non-owned use-cases,
the lifetime of these object was not properly modeled.
The line_info from an event may only be used for as long as the event
exists.
This allowed us to write unsafe-free Rust code that causes a
use-after-free:
let event = chip.read_info_event().unwrap();
let line_info = event.line_info().unwrap();
drop(event);
dbg!(line_info.name().unwrap());
Which makes the AddressSanitizer scream:
==90154==ERROR: AddressSanitizer: heap-use-after-free on address 0x50b000005dc4 at pc 0x55a4f883a009 bp 0x7f60ac8fbbc0 sp 0x7f60ac8fb388
READ of size 2 at 0x50b000005dc4 thread T2
[...]
#3 0x55a4f8c3d5f3 in libgpiod::line_info::Info::name::h5ba0bfd360ecb405 libgpiod/bindings/rust/libgpiod/src/line_info.rs:70:18
[...]
0x50b000005dc4 is located 4 bytes inside of 112-byte region [0x50b000005dc0,0x50b000005e30)
freed by thread T2 here:
[...]
#1 0x7f60b07f7e31 in gpiod_info_event_free libgpiod/lib/info-event.c:61:2
[...]
previously allocated by thread T2 here:
#0 0x55a4f88b04be in malloc /rustc/llvm/src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
#1 0x7f60b07f8ff0 in gpiod_line_info_from_uapi libgpiod/lib/line-info.c:144:9
The fix is to distinguish between the owned and non-owned variants and
assigning lifetimes to non-owned variants.
For modeling the non-owned type there are a couple of options. The ideal
solution would be using extern_types [1]. But that is still unstable.
Instead, we are defining a #[repr(transparent)] wrapper around the opaque
gpiod_line_info struct and cast the pointer to a reference.
This was recommended on the Rust Discord server as good practise.
(Thanks to Kyuuhachi, shepmaster, pie_flavor and ilyvion! Also thanks to
@epilys for a brainstorming on this on #linaro-virtualization IRC).
Of course, determining the lifetimes and casting across the types
requires some care. So this adds a couple of SAFETY comments that would
probably also have helped the existing code.
[1] https://github.com/rust-lang/rfcs/blob/master/text/1861-extern-types.md
Fixes: 91f9373c65 ("bindings: rust: Add libgpiod crate")
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The thread-safety rules of libgpiod allow individual object instances to
be used from different threads. So far, this was not actually possible
with the Rust bindings. Not being `Send` disallowed the user to transfer
the ownership to different threads.
Rust also has a `Sync` marker. That one would even allow sending
references of objects to other threads. Since we wrap a lot of C
functions with `fn foo(&self)` signatures, that would not be safe.
libgpiod does not allow concurrent API calls to the same object instance
- which Rust would allow for read-only references. Thus, we do not
define that one.
Chip was already modeled correctly.
line::Info is not marked as Send since it may either be owning or non-
owning. That problem is fixed as part of a separate pull request [1].
[1] https://lore.kernel.org/r/20230927-rust-line-info-soundness-v1-0-990dce6f18ab@linaro.org
Link: https://lore.kernel.org/r/CVHO091CC80Y.3KUOSLSOBVL0T@ablu-work
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Chip was modeled with an Arc that only was used to pass the chip pointer
to the chip::Info constructor. With that refactored to take a reference,
we can just drop the Arc.
This allows to get rid of the `Internal` helper struct that was only
required by the Arc.
As a side-effect, we also get rid of this clippy warning:
warning: usage of an `Arc` that is not `Send` or `Sync`
--> libgpiod/src/chip.rs:75:21
|
75 | let ichip = Arc::new(Internal::open(path)?);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the trait `Send` is not implemented for `Internal`
= note: the trait `Sync` is not implemented for `Internal`
= note: required for `Arc<Internal>` to implement `Send` and `Sync`
= help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reported by 1.74.0-nightly:
warning: call to `.clone()` on a reference in this situation does nothing
--> libgpiod/tests/line_request.rs:71:44
|
71 | let chip_name = sim.chip_name().clone();
| ^^^^^^^^ help: remove this redundant call
|
= note: the type `str` does not implement `Clone`, so calling `clone` on `&str` copies the reference, which does not do anything and can be removed
= note: `#[warn(noop_method_call)]` on by default
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Fix the following warning:
"some crates are on edition 2021 which defaults to `resolver = "2"`,
but virtual workspaces default to `resolver = "1"`"
Clarify the resolver selection as the 2021 edition by setting the
workspace.resolver to "2".
Signed-off-by: Kent Gibson <warthog618@gmail.com>
Reviewed-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Make the file comment for reconfigure_input_to_output consistent with
other language examples.
Signed-off-by: Kent Gibson <warthog618@gmail.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Replace tool examples with use case examples drawn from the tools,
gpio_events example with buffered_event_lifetimes, and
gpio_threaded_info_events with reconfigure_input_to_output.
Signed-off-by: Kent Gibson <warthog618@gmail.com>
Reviewed-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
A collection of minor changes to be more consistent with other examples:
- capitalize comments
- add line offset to value outputs
- drop comma from edge event outputs
Signed-off-by: Kent Gibson <warthog618@gmail.com>
Reviewed-by: Erik Schilling <erik.schilling@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Specifying MSRV (minimum supported rust version) was introduced with
Rust 1.56. So old versions of libraries do not have this restriction
spelled out in their Cargo.toml.
This means that even if we claimed a MSRV of 1.56 until now, that did
not mean that things were actually buildable with 1.56 practically
(without manually researching compatible versions and pinning them
down).
`bindgen` started listing a MSRV from v0.61 (requiring Rust 1.57) [1].
So that may seem like an obvious choice. But if one attempts to build
it with such an old version one will realize that `log` only started
requesting a MSRV with 0.4.19 (requesting Rust 1.60) [2]. Hence, we
would either need to manually restrict log to an old, but compatible
release (which would unnecessarily restrict everyone on newer Rust
versions) or just also bump our MSRV to 1.60.
Rust 1.60 was released on 2022-04-07 [3] and seems like an acceptable
choice if core components such as `log` started their MSRV tracking with
that version. If someone is determined wanting to use this with an older
version of Rust, thats still possible. If one has done the necessary
manual research and pinned their versions down, one can use
--ignore-rust-version (or a < 1.56 version of Rust that does not check
the MSRV yet).
Thanks to Manos Pitsidianakis <manos.pitsidianakis@linaro.org> for
helping me out when I successfully confused myself somewhere.
[1] https://github.com/rust-lang/rust-bindgen/blob/v0.61.0/bindgen/Cargo.toml
[2] https://github.com/rust-lang/log/blob/0.4.19/Cargo.toml
[3] https://blog.rust-lang.org/2022/04/07/Rust-1.60.0.html
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add new example files to EXTRA_DIST in the Makefile so that they end up
being shipped with the rest of the code.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
This change replaces building against "bundled" headers by always
building agains system headers (while following standard conventions to
allow users to specify the version to build against).
Reasoning:
Previously, the code generated the bindings based on the headers, but
then links against `-lgpiod` without further specifying where that is
coming from.
This results in some challenges and problems:
1. Packaging a Rust crate with `cargo package` requires the folder
containing the Cargo.toml to be self-contained. Essentially, a tar
ball with all the sources of that folder is created. Building against
that tar ball fails, since the headers files passed to bindgen are
a relative path pointing outside of that folder.
2. While, for example, the cxx bindings are built AND linked against
the build results, the packaging situation for C++ libraries is a
bit different compared to Rust libs. The C++ libs will likely get
built as part of the larger libgpiod build and published together
with the C variant.
In Rust, the vast majority of people will want to build the glue-code
during the compilation of the applications that consume this lib.
This may lead to inconsistencies between the bundled headers and the
libraries shipped by the user's distro. While ABI should hopefully
be forward-compatible within the same MAJOR number of the .so,
using too new headers will likely quickly lead to mismatches with
symbols defined in the lib.
3. Trying to build the core lib as part of the Rust build quickly runs
into similar packaging issues as the existing solution. The source
code of the C lib would need to become part of some package
(often people opt to pull it in as a submodule under their -sys crate
or even create a separate -src package [1]). This clearly does not
work well with the current setup...
Since building against system libs is probably? what 90%+ of the people
want, this change hopefully addresses the problems above. The
system-deps dependency honors pkg-config conventions, but also allows
users flexible ways to override the defaults [2]. Overall, this keeps
things simple while still allowing maximum flexibility.
Since the pkg-config interface is just telling us which include paths to
use, we switch back to a wrapper.h file that includes the real gpiod.h.
Once Rust bindings require a lower version floor, the version metadata
can also be updated to help telling users that their system library is
too old.
In order to support people hacking on the Rust bindings, building with
make will automatically set the right set of environment variables.
In case people want to customize it when building without `make`, a
reference to the system_deps documentation is given in the README.md.
[1] https://github.com/alexcrichton/openssl-src-rs
[2] https://docs.rs/system-deps/latest/system_deps/#overriding-build-flags
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
We don't really need to build all the available rust crates here by
default, but only what's required by the libgpiod crate.
Currently we try to build gpiosim-sys crate as well, even if
"--enable-tests" isn't passed to autogen.sh, which results in following
build failure:
error: could not find native static library `gpiosim`, perhaps an -L flag is missing?
Fix this by building just the libgpiod crate instead, which can force
building of the other crates based on the enabled configuration options.
Reported-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The currently selected version of bindgen has a unmaintained dependency
and generates the following build warning while building vhost-device
crate:
Crate: ansi_term
Version: 0.12.1
Warning: unmaintained
Title: ansi_term is Unmaintained
Date: 2021-08-18
ID: RUSTSEC-2021-0139
URL: https://rustsec.org/advisories/RUSTSEC-2021-0139
Dependency tree:
ansi_term 0.12.1
└── clap 2.34.0
└── bindgen 0.59.2
└── libgpiod-sys 0.1.0
└── libgpiod 0.1.0
└── vhost-device-gpio 0.1.0
error: 1 denied warning found!
Fix it by moving to a later version of bindgen, which updates the types
of few of the arguments to the FFI helpers and so required changes to
few of explicit type conversions.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add a new function to line config allowing to set a list of output values
for requested lines. This works very similarily to the C++ version of the
new C interface.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
In C++ bindings we can chain the mutators as they all return a reference
to the object they modify. It's a common practice to allow that in Rust
too so make all mutators that don't already do it return a mutable
reference to self.
It's also logically incorrect to make mutators borrow an immutable
reference to self. Even if that builds - as we're fiddling with C
pointers - it could change in the future. It's fine for getters but
setters should all use mutable references.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
We have two functions in the C API that allow users to retrieve a list
of offsets from objects: gpiod_line_request_get_offsets() and
gpiod_line_config_get_offsets(). Even though they serve pretty much the
same purpose, they have different signatures and one of them also
requires the user to free the memory allocated within the libgpiod
library with a non-libgpiod free() function.
Unify them: make them take the array in which to store offsets and the
size of this array. Make them return the number of offsets actually
stored in the array and make them impossible to fail. Change their names
to be more descriptive and in the case of line_config: add a new function
that allows users to get the number of configured offsets.
Update the entire tree to use the new interfaces.
For rust bindings: also unify the line config interface to return a map
of line settings like C++ bindings do instead of having a function to
get settings by offset. A map returned from a single call is easier to
iterate over with a for loop than using an integer and calling the
previous line_settings() method.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
