* [PATCH libgpiod 1/2] bindings: rust: mark constructors that take raw pointers unsafe
2025-07-21 7:41 [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename to `from_raw` Erik Wierich
@ 2025-07-21 7:41 ` Erik Wierich
2025-07-21 7:41 ` [PATCH libgpiod 2/2] bindings: rust: rename constructors that wrap raw objects to `from_raw` Erik Wierich
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Erik Wierich @ 2025-07-21 7:41 UTC (permalink / raw)
To: Linux-GPIO, Bartosz Golaszewski, Viresh Kumar; +Cc: Erik Wierich
The functions take raw pointers and expose safe API that operates on
them. Thus, invariants have to be upheld by these constructors to ensure
safe operation of later API.
Signed-off-by: Erik Wierich <erik@riscstar.com>
---
bindings/rust/libgpiod/src/chip.rs | 10 ++++++++--
bindings/rust/libgpiod/src/info_event.rs | 7 ++++++-
bindings/rust/libgpiod/src/line_config.rs | 7 ++++++-
bindings/rust/libgpiod/src/line_request.rs | 7 ++++++-
bindings/rust/libgpiod/src/line_settings.rs | 10 +++++++++-
5 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/bindings/rust/libgpiod/src/chip.rs b/bindings/rust/libgpiod/src/chip.rs
index bbb962f0390469b3193deec9388b5b56efab07ce..f39beab58363beaebe2ba2dcfb846178b7b595e5 100644
--- a/bindings/rust/libgpiod/src/chip.rs
+++ b/bindings/rust/libgpiod/src/chip.rs
@@ -146,15 +146,18 @@ impl Chip {
if event.is_null() {
return Err(Error::OperationFailed(
OperationType::ChipReadInfoEvent,
errno::errno(),
));
}
- Ok(info::Event::new(event))
+ // SAFETY: `gpiod_chip_read_info_event` returned a standalone `event`
+ // over which we have sole ownership. We won't use the raw pointer
+ // directly after passing it here.
+ Ok(unsafe { info::Event::new(event) })
}
/// Map a GPIO line's name to its offset within the chip.
pub fn line_offset_from_name(&self, name: &str) -> Result<Offset> {
let name = CString::new(name).map_err(|_| Error::InvalidString)?;
// SAFETY: `gpiod_chip` is guaranteed to be valid here.
@@ -191,15 +194,18 @@ impl Chip {
if request.is_null() {
return Err(Error::OperationFailed(
OperationType::ChipRequestLines,
errno::errno(),
));
}
- request::Request::new(request)
+ // SAFETY: `gpiod_chip_request_lines` returned an object over which we
+ // have sole ownership. We never use it again after constructing the
+ // wrapper.
+ unsafe { request::Request::new(request) }
}
}
impl Drop for Chip {
/// Close the chip and release all associated resources.
fn drop(&mut self) {
// SAFETY: `gpiod_chip` is guaranteed to be valid here.
diff --git a/bindings/rust/libgpiod/src/info_event.rs b/bindings/rust/libgpiod/src/info_event.rs
index 472c8915a316f6dfdd184ff52e33754c0805f880..e59de89a0507aa224fc5f473f321a0817fe29980 100644
--- a/bindings/rust/libgpiod/src/info_event.rs
+++ b/bindings/rust/libgpiod/src/info_event.rs
@@ -27,15 +27,20 @@ pub struct Event {
// SAFETY: Event models a wrapper around an owned gpiod_info_event and may be
// safely sent to other threads.
unsafe impl Send for Event {}
impl Event {
/// Get a single chip's line's status change event.
- pub(crate) fn new(event: *mut gpiod::gpiod_info_event) -> Self {
+ ///
+ /// SAFETY: The pointer must point to an instance that is valid. After
+ /// constructing an [Event] the pointer MUST NOT be used for any other
+ /// purpose anymore. All interactions with the libgpiod API have to happen
+ /// through this object.
+ pub(crate) unsafe fn new(event: *mut gpiod::gpiod_info_event) -> Self {
Self { event }
}
/// Get the event type of the status change event.
pub fn event_type(&self) -> Result<InfoChangeKind> {
// SAFETY: `gpiod_info_event` is guaranteed to be valid here.
InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) })
diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs
index d0a4abae5ae7e0d5e111cf7778eaf3cd0c07ca53..09a65514748d0a85fb39a3d37a18f19fc9f4af1b 100644
--- a/bindings/rust/libgpiod/src/line_config.rs
+++ b/bindings/rust/libgpiod/src/line_config.rs
@@ -130,15 +130,20 @@ impl Config {
if settings.is_null() {
return Err(Error::OperationFailed(
OperationType::LineConfigGetSettings,
errno::errno(),
));
}
- map.insert(*offset as u64, Settings::new_with_settings(settings));
+ // SAFETY: The above `gpiod_line_config_get_line_settings` call
+ // returns a copy of the line_settings. We thus have sole ownership.
+ // We no longer use the pointer for any other purpose.
+ let settings = unsafe { Settings::new_with_settings(settings) };
+
+ map.insert(*offset as u64, settings);
}
Ok(map)
}
}
impl Drop for Config {
diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs
index d7b62a1a083ff589aaa4fa3edb1b1269e7aafaf5..c73943da9b42a12b35adf5234c80e62f4c7785a7 100644
--- a/bindings/rust/libgpiod/src/line_request.rs
+++ b/bindings/rust/libgpiod/src/line_request.rs
@@ -23,15 +23,20 @@ pub struct Request {
// SAFETY: Request models a wrapper around an owned gpiod_line_request and may
// be safely sent to other threads.
unsafe impl Send for Request {}
impl Request {
/// Request a set of lines for exclusive usage.
- pub(crate) fn new(request: *mut gpiod::gpiod_line_request) -> Result<Self> {
+ ///
+ /// SAFETY: The pointer must point to an instance that is valid. After
+ /// constructing a [Request] the pointer MUST NOT be used for any other
+ /// purpose anymore. All interactions with the libgpiod API have to happen
+ /// through this object.
+ pub(crate) unsafe fn new(request: *mut gpiod::gpiod_line_request) -> Result<Self> {
Ok(Self { request })
}
/// Get the name of the chip this request was made on.
#[cfg(feature = "v2_1")]
pub fn chip_name(&self) -> Result<&str> {
// SAFETY: The `gpiod_line_request` is guaranteed to be live as long
diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
index 7f271c3b7397c81d4d2e15fc8050582306ad2b5b..6445a4756b1f12932e266148a4ac9528c7ac1d06 100644
--- a/bindings/rust/libgpiod/src/line_settings.rs
+++ b/bindings/rust/libgpiod/src/line_settings.rs
@@ -42,15 +42,23 @@ impl Settings {
errno::errno(),
));
}
Ok(Self { settings })
}
- pub(crate) fn new_with_settings(settings: *mut gpiod::gpiod_line_settings) -> Self {
+ /// Converts a owned pointer into an owned instance
+ ///
+ /// Assumes sole ownership over a [gpiod::gpiod_line_settings] instance.
+ ///
+ /// SAFETY: The pointer must point to an instance that is valid. After
+ /// constructing a [Settings] the pointer MUST NOT be used for any other
+ /// purpose anymore. All interactions with the libgpiod API have to happen
+ /// through this object.
+ pub(crate) unsafe fn new_with_settings(settings: *mut gpiod::gpiod_line_settings) -> Self {
Self { settings }
}
/// Resets the line settings object to its default values.
pub fn reset(&mut self) {
// SAFETY: `gpiod_line_settings` is guaranteed to be valid here.
unsafe { gpiod::gpiod_line_settings_reset(self.settings) }
--
2.50.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH libgpiod 2/2] bindings: rust: rename constructors that wrap raw objects to `from_raw`
2025-07-21 7:41 [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename to `from_raw` Erik Wierich
2025-07-21 7:41 ` [PATCH libgpiod 1/2] bindings: rust: mark constructors that take raw pointers unsafe Erik Wierich
@ 2025-07-21 7:41 ` Erik Wierich
2025-07-21 7:48 ` [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename " Viresh Kumar
2025-07-21 13:20 ` Bartosz Golaszewski
3 siblings, 0 replies; 5+ messages in thread
From: Erik Wierich @ 2025-07-21 7:41 UTC (permalink / raw)
To: Linux-GPIO, Bartosz Golaszewski, Viresh Kumar; +Cc: Erik Wierich
This matches the stdlib conventions[1] and seems easier to read to me.
[1] https://doc.rust-lang.org/stable/std/?search=from_raw
Signed-off-by: Erik Wierich <erik@riscstar.com>
---
bindings/rust/libgpiod/src/chip.rs | 4 ++--
bindings/rust/libgpiod/src/info_event.rs | 2 +-
bindings/rust/libgpiod/src/line_config.rs | 2 +-
bindings/rust/libgpiod/src/line_request.rs | 2 +-
bindings/rust/libgpiod/src/line_settings.rs | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/bindings/rust/libgpiod/src/chip.rs b/bindings/rust/libgpiod/src/chip.rs
index f39beab58363beaebe2ba2dcfb846178b7b595e5..df1cef3bdef5d2014e951437c9fe89c119bff48c 100644
--- a/bindings/rust/libgpiod/src/chip.rs
+++ b/bindings/rust/libgpiod/src/chip.rs
@@ -149,15 +149,15 @@ impl Chip {
errno::errno(),
));
}
// SAFETY: `gpiod_chip_read_info_event` returned a standalone `event`
// over which we have sole ownership. We won't use the raw pointer
// directly after passing it here.
- Ok(unsafe { info::Event::new(event) })
+ Ok(unsafe { info::Event::from_raw(event) })
}
/// Map a GPIO line's name to its offset within the chip.
pub fn line_offset_from_name(&self, name: &str) -> Result<Offset> {
let name = CString::new(name).map_err(|_| Error::InvalidString)?;
// SAFETY: `gpiod_chip` is guaranteed to be valid here.
@@ -197,15 +197,15 @@ impl Chip {
errno::errno(),
));
}
// SAFETY: `gpiod_chip_request_lines` returned an object over which we
// have sole ownership. We never use it again after constructing the
// wrapper.
- unsafe { request::Request::new(request) }
+ unsafe { request::Request::from_raw(request) }
}
}
impl Drop for Chip {
/// Close the chip and release all associated resources.
fn drop(&mut self) {
// SAFETY: `gpiod_chip` is guaranteed to be valid here.
diff --git a/bindings/rust/libgpiod/src/info_event.rs b/bindings/rust/libgpiod/src/info_event.rs
index e59de89a0507aa224fc5f473f321a0817fe29980..f23701ab43ce5c215bb5260cd608d8e46b927079 100644
--- a/bindings/rust/libgpiod/src/info_event.rs
+++ b/bindings/rust/libgpiod/src/info_event.rs
@@ -32,15 +32,15 @@ unsafe impl Send for Event {}
impl Event {
/// Get a single chip's line's status change event.
///
/// SAFETY: The pointer must point to an instance that is valid. After
/// constructing an [Event] the pointer MUST NOT be used for any other
/// purpose anymore. All interactions with the libgpiod API have to happen
/// through this object.
- pub(crate) unsafe fn new(event: *mut gpiod::gpiod_info_event) -> Self {
+ pub(crate) unsafe fn from_raw(event: *mut gpiod::gpiod_info_event) -> Self {
Self { event }
}
/// Get the event type of the status change event.
pub fn event_type(&self) -> Result<InfoChangeKind> {
// SAFETY: `gpiod_info_event` is guaranteed to be valid here.
InfoChangeKind::new(unsafe { gpiod::gpiod_info_event_get_event_type(self.event) })
diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs
index 09a65514748d0a85fb39a3d37a18f19fc9f4af1b..34b6c227b0c8e156ea1bac396cc19ea4f182012c 100644
--- a/bindings/rust/libgpiod/src/line_config.rs
+++ b/bindings/rust/libgpiod/src/line_config.rs
@@ -133,15 +133,15 @@ impl Config {
errno::errno(),
));
}
// SAFETY: The above `gpiod_line_config_get_line_settings` call
// returns a copy of the line_settings. We thus have sole ownership.
// We no longer use the pointer for any other purpose.
- let settings = unsafe { Settings::new_with_settings(settings) };
+ let settings = unsafe { Settings::from_raw(settings) };
map.insert(*offset as u64, settings);
}
Ok(map)
}
}
diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs
index c73943da9b42a12b35adf5234c80e62f4c7785a7..49fe56542ab876bd2360b5e846e18ced0de51fbd 100644
--- a/bindings/rust/libgpiod/src/line_request.rs
+++ b/bindings/rust/libgpiod/src/line_request.rs
@@ -28,15 +28,15 @@ unsafe impl Send for Request {}
impl Request {
/// Request a set of lines for exclusive usage.
///
/// SAFETY: The pointer must point to an instance that is valid. After
/// constructing a [Request] the pointer MUST NOT be used for any other
/// purpose anymore. All interactions with the libgpiod API have to happen
/// through this object.
- pub(crate) unsafe fn new(request: *mut gpiod::gpiod_line_request) -> Result<Self> {
+ pub(crate) unsafe fn from_raw(request: *mut gpiod::gpiod_line_request) -> Result<Self> {
Ok(Self { request })
}
/// Get the name of the chip this request was made on.
#[cfg(feature = "v2_1")]
pub fn chip_name(&self) -> Result<&str> {
// SAFETY: The `gpiod_line_request` is guaranteed to be live as long
diff --git a/bindings/rust/libgpiod/src/line_settings.rs b/bindings/rust/libgpiod/src/line_settings.rs
index 6445a4756b1f12932e266148a4ac9528c7ac1d06..9ab8ea7173c4f214e6d513434435a144424ecc74 100644
--- a/bindings/rust/libgpiod/src/line_settings.rs
+++ b/bindings/rust/libgpiod/src/line_settings.rs
@@ -50,15 +50,15 @@ impl Settings {
///
/// Assumes sole ownership over a [gpiod::gpiod_line_settings] instance.
///
/// SAFETY: The pointer must point to an instance that is valid. After
/// constructing a [Settings] the pointer MUST NOT be used for any other
/// purpose anymore. All interactions with the libgpiod API have to happen
/// through this object.
- pub(crate) unsafe fn new_with_settings(settings: *mut gpiod::gpiod_line_settings) -> Self {
+ pub(crate) unsafe fn from_raw(settings: *mut gpiod::gpiod_line_settings) -> Self {
Self { settings }
}
/// Resets the line settings object to its default values.
pub fn reset(&mut self) {
// SAFETY: `gpiod_line_settings` is guaranteed to be valid here.
unsafe { gpiod::gpiod_line_settings_reset(self.settings) }
--
2.50.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename to `from_raw`
2025-07-21 7:41 [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename to `from_raw` Erik Wierich
2025-07-21 7:41 ` [PATCH libgpiod 1/2] bindings: rust: mark constructors that take raw pointers unsafe Erik Wierich
2025-07-21 7:41 ` [PATCH libgpiod 2/2] bindings: rust: rename constructors that wrap raw objects to `from_raw` Erik Wierich
@ 2025-07-21 7:48 ` Viresh Kumar
2025-07-21 13:20 ` Bartosz Golaszewski
3 siblings, 0 replies; 5+ messages in thread
From: Viresh Kumar @ 2025-07-21 7:48 UTC (permalink / raw)
To: Erik Wierich; +Cc: Linux-GPIO, Bartosz Golaszewski
On 21-07-25, 09:41, Erik Wierich wrote:
> This is based on top of:
> https://lore.kernel.org/linux-gpio/20250720-new-with-settings-crate-v1-1-a51392bd5b13@linaro.org/
>
> Signed-off-by: Erik Wierich <erik@riscstar.com>
> ---
> Erik Wierich (2):
> bindings: rust: mark constructors that take raw pointers unsafe
> bindings: rust: rename constructors that wrap raw objects to `from_raw`
>
> bindings/rust/libgpiod/src/chip.rs | 10 ++++++++--
> bindings/rust/libgpiod/src/info_event.rs | 7 ++++++-
> bindings/rust/libgpiod/src/line_config.rs | 7 ++++++-
> bindings/rust/libgpiod/src/line_request.rs | 7 ++++++-
> bindings/rust/libgpiod/src/line_settings.rs | 10 +++++++++-
> 5 files changed, 35 insertions(+), 6 deletions(-)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename to `from_raw`
2025-07-21 7:41 [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename to `from_raw` Erik Wierich
` (2 preceding siblings ...)
2025-07-21 7:48 ` [PATCH libgpiod 0/2] bindings: rust: mark raw constructors as unsafe + rename " Viresh Kumar
@ 2025-07-21 13:20 ` Bartosz Golaszewski
3 siblings, 0 replies; 5+ messages in thread
From: Bartosz Golaszewski @ 2025-07-21 13:20 UTC (permalink / raw)
To: Linux-GPIO, Bartosz Golaszewski, Viresh Kumar, Erik Wierich
Cc: Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On Mon, 21 Jul 2025 09:41:52 +0200, Erik Wierich wrote:
> This is based on top of:
> https://lore.kernel.org/linux-gpio/20250720-new-with-settings-crate-v1-1-a51392bd5b13@linaro.org/
>
>
Applied, thanks!
[1/2] bindings: rust: mark constructors that take raw pointers unsafe
commit: 1651710b378f7d8bf2fdf647c7f992828fdcea6d
[2/2] bindings: rust: rename constructors that wrap raw objects to `from_raw`
commit: f1a949f8276eccbe8ad838adf7f9ed9898aba5f7
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread