core: provide information about the parent chip in line requests
Subsystems (2)
v2 cover core: provide information about the parent chip in line requests
From: Bartosz Golaszewski <[email protected]>
While working on the DBus API, it occurred to me that while we can obtain the list of requested offsets from a line request, this information lacks context if we cannot get any information about the parent chip on which the request was made.
We cannot reference the chip in any way as its lifetime is disconnected from the request but we can at least provide the name of the chip used to create the request providing some context for the offsets.
This series adds a new getter for struct gpiod_line_request and wrappers for it for all bindings. This will be used in the upcoming DBus GPIO manager code.
v1 -> v2:
- provide the chip name, not its filesystem path
- fix UNSAFE comments in rust bindings
Bartosz Golaszewski (5):
core: provide gpiod_line_request_get_chip_name()
tests: add a test-case for gpiod_line_request_get_chip_name()
bindings: cxx: provide line_request::chip_name()
bindings: python: provide the chip_name property in line_request
bindings: rust: provide LineRequest::chip_name()
bindings/cxx/gpiodcxx/line-request.hpp | 6 ++++++ bindings/cxx/line-request.cpp | 10 +++++++++- bindings/cxx/tests/tests-line-request.cpp | 6 ++++-- bindings/python/gpiod/chip.py | 1 + bindings/python/gpiod/ext/request.c | 11 ++++++++++ bindings/python/gpiod/line_request.py | 12 +++++++++-- bindings/python/tests/tests_line_request.py | 13 +++++++----- bindings/rust/libgpiod/src/line_request.rs | 16 +++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 +++++++++++++ include/gpiod.h | 9 +++++++++ lib/chip.c | 7 ++++++- lib/internal.h | 3 ++- lib/line-request.c | 20 ++++++++++++++++++- tests/tests-line-request.c | 21 ++++++++++++++++++++ 14 files changed, 136 insertions(+), 13 deletions(-)
Signature · 1 line
2.39.2
PATCH v2 1/5 core: provide gpiod_line_request_get_chip_name()
From: Bartosz Golaszewski <[email protected]>
While we can get the list of requested offsets from a line-request object, this information lacks context if we don't provide any data about the GPIO chip the request was made on. Add a helper allowing users to get the name of the parent chip.
--- include/gpiod.h | 9 +++++++++ lib/chip.c | 7 ++++++- lib/internal.h | 3 ++- lib/line-request.c | 20 +++++++++++++++++++- 4 files changed, 36 insertions(+), 3 deletions(-)
include/gpiod.h +9 -0
@@ -1007,6 +1007,15 @@ gpiod_request_config_get_event_buffer_size(struct gpiod_request_config *config);*/ void gpiod_line_request_release (struct gpiod_line_request *request); +/** +* @brief Get the name of the chip this request was made on. +* @param request Line request object. +* @return Name the GPIO chip device. The returned pointer is valid for the +* lifetime of the request object and must not be freed by the caller. +*/ +const char * +gpiod_line_request_get_chip_name (struct gpiod_line_request *request); +/** * @brief Get the number of lines in the request. * @param request Line request object.
lib/chip.c +6 -1
@@ -215,6 +215,7 @@ gpiod_chip_request_lines(struct gpiod_chip *chip, {struct gpio_v2_line_request uapi_req;struct gpiod_line_request *request; +struct gpiochip_info info;int ret;assert (chip);
@@ -233,11 +234,15 @@ gpiod_chip_request_lines(struct gpiod_chip *chip,if (ret)return NULL ; + ret =read_chip_info (chip->fd, &info); +if (ret <0 ) +return NULL ; + ret =ioctl (chip->fd,GPIO_V2_GET_LINE_IOCTL , &uapi_req);if (ret <0 )return NULL ; - request =gpiod_line_request_from_uapi (&uapi_req); + request =gpiod_line_request_from_uapi (&uapi_req, info.name);if (!request) {close (uapi_req.fd);return NULL ;
lib/internal.h +2 -1
@@ -26,7 +26,8 @@ void gpiod_request_config_to_uapi(struct gpiod_request_config *config,int gpiod_line_config_to_uapi (struct gpiod_line_config *config,struct gpio_v2_line_request *uapi_cfg);struct gpiod_line_request * -gpiod_line_request_from_uapi (struct gpio_v2_line_request *uapi_req); +gpiod_line_request_from_uapi (struct gpio_v2_line_request *uapi_req, +const char *chip_name);int gpiod_edge_event_buffer_read_fd (int fd,struct gpiod_edge_event_buffer *buffer,size_t max_events);
lib/line-request.c +19 -1
@@ -13,13 +13,15 @@#include "internal.h" struct gpiod_line_request { +char *chip_name;unsigned int offsets[GPIO_V2_LINES_MAX ];size_t num_lines;int fd; };struct gpiod_line_request * -gpiod_line_request_from_uapi (struct gpio_v2_line_request *uapi_req) +gpiod_line_request_from_uapi (struct gpio_v2_line_request *uapi_req, +const char *chip_name) {struct gpiod_line_request *request;
@@ -28,6 +30,13 @@ gpiod_line_request_from_uapi(struct gpio_v2_line_request *uapi_req)return NULL ;memset (request,0 ,sizeof (*request)); + + request->chip_name =strdup (chip_name); +if (!request->chip_name) { +free (request); +return NULL ; + } + request->fd = uapi_req->fd; request->num_lines = uapi_req->num_lines; memcpy(request->offsets, uapi_req->offsets,
@@ -42,9 +51,18 @@ GPIOD_API void gpiod_line_request_release(struct gpiod_line_request *request)return ;close (request->fd); +free (request->chip_name);free (request); } +GPIOD_API const char * +gpiod_line_request_get_chip_name (struct gpiod_line_request *request) +{ +assert (request); + +return request->chip_name; +} + GPIOD_API size_tgpiod_line_request_get_num_requested_lines (struct gpiod_line_request *request) {
Signature · 1 line
2.39.2
PATCH v2 3/5 bindings: cxx: provide line_request::chip_name()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Provide a wrapper around gpiod_line_request_get_chip_name() for C++ bindings and update the tests.
--- bindings/cxx/gpiodcxx/line-request.hpp | 6 ++++++ bindings/cxx/line-request.cpp | 10 +++++++++- bindings/cxx/tests/tests-line-request.cpp | 6 ++++-- 3 files changed, 19 insertions(+), 3 deletions(-)
bindings/cxx/gpiodcxx/line-request.hpp +6 -0
@@ -75,6 +75,12 @@ public: */ void release(); + /** + * @brief Get the name of the chip this request was made on. + * @return Name to the GPIO chip. + */ + ::std::string chip_name() const; + /** * @brief Get the number of requested lines. * @return Number of lines in this request.
bindings/cxx/line-request.cpp +9 -1
@@ -63,6 +63,13 @@ GPIOD_CXX_API void line_request::release() this->_m_priv->request.reset(); } +GPIOD_CXX_API ::std::string line_request::chip_name() const +{ + this->_m_priv->throw_if_released(); + + return ::gpiod_line_request_get_chip_name(this->_m_priv->request.get()); +} + GPIOD_CXX_API ::std::size_t line_request::num_lines() const { this->_m_priv->throw_if_released();
@@ -222,7 +229,8 @@ GPIOD_CXX_API ::std::ostream& operator<<(::std::ostream& out, const line_request if (!request) out << "gpiod::line_request(released)"; else - out << "gpiod::line_request(num_lines=" << request.num_lines() << + out << "gpiod::line_request(chip=\"" << request.chip_name() << + "\", num_lines=" << request.num_lines() << ", line_offsets=" << request.offsets() << ", fd=" << request.fd() << ")";
bindings/cxx/tests/tests-line-request.cpp +4 -2
@@ -468,14 +468,16 @@ TEST_CASE("line_request stream insertion operator works", "[line-request]") .set_num_lines(4) .build(); - auto request = ::gpiod::chip(sim.dev_path()) + auto chip = ::gpiod::chip(sim.dev_path()); + auto request = chip .prepare_request() .add_line_settings({ 3, 1, 0, 2}, ::gpiod::line_settings()) .do_request(); ::std::stringstream buf, expected; - expected << "gpiod::line_request(num_lines=4, line_offsets=gpiod::offsets(3, 1, 0, 2), fd=" << + expected << "gpiod::line_request(chip=\"" << sim.name() << + "\", num_lines=4, line_offsets=gpiod::offsets(3, 1, 0, 2), fd=" << request.fd() << ")"; SECTION("active request")
Signature · 1 line
2.39.2
PATCH v2 4/5 bindings: python: provide the chip_name property in line_request
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Provide a wrapper around gpiod_line_request_get_chip_name() for Python bindings and update the tests.
--- bindings/python/gpiod/chip.py | 1 + bindings/python/gpiod/ext/request.c | 11 +++++++++++ bindings/python/gpiod/line_request.py | 12 ++++++++++-- bindings/python/tests/tests_line_request.py | 13 ++++++++----- 4 files changed, 30 insertions(+), 7 deletions(-)
bindings/python/gpiod/chip.py +1 -0
@@ -315,6 +315,7 @@ class Chip: req_internal = self._chip.request_lines(line_cfg, consumer, event_buffer_size) request = LineRequest(req_internal) + request._chip_name = req_internal.chip_name request._offsets = req_internal.offsets request._name_map = name_map request._offset_map = offset_map
bindings/python/gpiod/ext/request.c +11 -0
@@ -37,6 +37,13 @@ static void request_finalize(request_object *self)gpiod_edge_event_buffer_free (self->buffer); } +static PyObject * +request_chip_name (request_object *self,void *Py_UNUSED (ignored )) +{ +return PyUnicode_FromString ( +gpiod_line_request_get_chip_name (self->request)); +} +static PyObject *request_num_lines (request_object *self,void *Py_UNUSED (ignored )) {
@@ -92,6 +99,10 @@ static PyObject *request_fd(request_object *self, void *Py_UNUSED(ignored)) }static PyGetSetDef request_getset[] = { + { + .name ="chip_name" , + .get = (getter )request_chip_name, + }, { .name ="num_lines" , .get = (getter )request_num_lines,
bindings/python/gpiod/line_request.py +10 -2
@@ -212,10 +212,18 @@ class LineRequest: if not self._req: return "<LineRequest RELEASED>" - return "<LineRequest num_lines={} offsets={} fd={}>".format( - self.num_lines, self.offsets, self.fd + return '<LineRequest chip="{}" num_lines={} offsets={} fd={}>'.format( + self.chip_name, self.num_lines, self.offsets, self.fd ) + @property + def chip_name(self) -> str: + """ + Name of the chip this request was made on. + """ + self._check_released() + return self._chip_name + @property def num_lines(self) -> int: """
bindings/python/tests/tests_line_request.py +8 -5
@@ -529,11 +529,14 @@ class LineRequestStringRepresentation(TestCase): del self.sim def test_str(self): - with gpiod.request_lines(self.sim.dev_path, config={(2, 6, 4, 1): None}) as req: - self.assertEqual( - str(req), - "<LineRequest num_lines=4 offsets=[2, 6, 4, 1] fd={}>".format(req.fd), - ) + with gpiod.Chip(self.sim.dev_path) as chip: + with chip.request_lines(config={(2, 6, 4, 1): None}) as req: + self.assertEqual( + str(req), + '<LineRequest chip="{}" num_lines=4 offsets=[2, 6, 4, 1] fd={}>'.format( + self.sim.name, req.fd + ), + ) def test_str_released(self): req = gpiod.request_lines(self.sim.dev_path, config={(2, 6, 4, 1): None})
Signature · 1 line
2.39.2
PATCH v2 2/5 tests: add a test-case for gpiod_line_request_get_chip_name()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Add a test case for the chip name getter on line-request objects.
--- tests/tests-line-request.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
tests/tests-line-request.c +21 -0
@@ -675,3 +675,24 @@ GPIOD_TEST_CASE(get_requested_offsets_less_and_more) g_assert_cmpuint(retrieved[2], ==, 2); g_assert_cmpuint(retrieved[3], ==, 3); } + +GPIOD_TEST_CASE (get_chip_name) +{ +static const guint offset =4 ; + +g_autoptr (GPIOSimChip) sim =g_gpiosim_chip_new ("num-lines" ,8 ,NULL ); +g_autoptr (struct_gpiod_chip) chip =NULL ; +g_autoptr (struct_gpiod_line_config) line_cfg =NULL ; +g_autoptr (struct_gpiod_line_request) request =NULL ; + + chip =gpiod_test_open_chip_or_fail (g_gpiosim_chip_get_dev_path (sim)); + line_cfg =gpiod_test_create_line_config_or_fail (); + +gpiod_test_line_config_add_line_settings_or_fail (line_cfg, &offset,1 , +NULL ); + + request =gpiod_test_chip_request_lines_or_fail (chip,NULL , line_cfg); + + g_assert_cmpstr(g_gpiosim_chip_get_name(sim), ==, +gpiod_line_request_get_chip_name (request)); +}
Signature · 1 line
2.39.2
PATCH v2 5/5 bindings: rust: provide LineRequest::chip_name()
In reply to Bartosz GolaszewskiFrom: Bartosz Golaszewski <[email protected]>
Provide a wrapper around gpiod_line_request_get_chip_name() for Rust bindings and add a test-case.
--- bindings/rust/libgpiod/src/line_request.rs | 16 ++++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++ 2 files changed, 30 insertions(+)
bindings/rust/libgpiod/src/line_request.rs +16 -0
@@ -2,6 +2,7 @@// SPDX-FileCopyrightText: 2022 Linaro Ltd. // SPDX-FileCopyrightText: 2022 Viresh Kumar <[email protected]> +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 ) + } +/// Get the number of lines in the request. pub fn num_lines (&self) ->usize {// SAFETY: `gpiod_line_request` is guaranteed to be valid here.
bindings/rust/libgpiod/tests/line_request.rs +14 -0
@@ -59,6 +59,20 @@ mod line_request {mod verify {use super ::*; +#[test] +fn chip_name () { +const GPIO :Offset =2 ; +let mut config =TestConfig ::new (NGPIO ).unwrap (); + config.lconfig_add_settings (&[GPIO ]); + config.request_lines ().unwrap (); + +let arc = config.sim (); +let sim = arc.lock ().unwrap (); +let chip_name = sim.chip_name ().clone (); + +assert_eq! (config.request ().chip_name ().unwrap (), chip_name); + } +#[test] fn custom_consumer () {const GPIO :Offset =2 ;
Signature · 1 line
2.39.2
Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
Quoted 10 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a wrapper around
gpiod_line_request_get_chip_name()for Rust bindings and add a test-case.Signed-off-by Bartosz Golaszewski <[email protected]>--- bindings/rust/libgpiod/src/line_request.rs | 16 ++++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++ 2 files changed, 30 insertions(+)
Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
In reply to Bartosz GolaszewskiQuoted 32 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a wrapper around
gpiod_line_request_get_chip_name()for Rust bindings and add a test-case.Signed-off-by Bartosz Golaszewski <[email protected]>--- bindings/rust/libgpiod/src/line_request.rs | 16 ++++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++ 2 files changed, 30 insertions(+)bindings/rust/libgpiod/src/line_request.rs +6
-0@@ -2,6 +2,7 @@// SPDX-FileCopyrightText: 2022 Linaro Ltd. // SPDX-FileCopyrightText: 2022 Viresh Kumar <[email protected]> +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) };
bindings/rust/libgpiod/src/line_request.rs impl Request { · line 43
Quoted 10 lines
bindings/rust/libgpiod/src/line_request.rs +10
-0+ +// 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.
Other than that the series looks good to me.
Cheers,
Kent.
Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
In reply to Bartosz GolaszewskiQuoted 5 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a wrapper around
gpiod_line_request_get_chip_name()for Rust bindings and add a test-case.
One other thing I noticed - just a heads up that the bindings generate a
new clippy warning from lastest nightly:
(rustc 1.73.0-nightly (399b06823 2023-07-20) (from rustc 1.73.0-nightly (ad963232d 2023-07-14))
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
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
= note: #[warn(clippy::arc_with_non_send_sync)] on by default
warning: libgpiod (lib) generated 1 warning
Finished dev [unoptimized + debuginfo] target(s) in 11.01s
That is independent of this patch.
And it is the nightly, which is a WIP so that may change - I noticed an older nightly reported this as an error.
Cheers,
Kent.
Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
In reply to Bartosz GolaszewskiQuoted 10 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a wrapper around
gpiod_line_request_get_chip_name()for Rust bindings and add a test-case.Signed-off-by Bartosz Golaszewski <[email protected]>--- bindings/rust/libgpiod/src/line_request.rs | 16 ++++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++ 2 files changed, 30 insertions(+)
Signature · 1 line
viresh
Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
In reply to Kent GibsonQuoted 38 lines · Kent Gibson
Quoted 36 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a wrapper around
gpiod_line_request_get_chip_name()for Rust bindings and add a test-case.Signed-off-by Bartosz Golaszewski <[email protected]>--- bindings/rust/libgpiod/src/line_request.rs | 16 ++++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++ 2 files changed, 30 insertions(+)bindings/rust/libgpiod/src/line_request.rs +10
-0@@ -2,6 +2,7 @@// SPDX-FileCopyrightText: 2022 Linaro Ltd. // SPDX-FileCopyrightText: 2022 Viresh Kumar <[email protected]> +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
Quoted 9 lines
bindings/rust/libgpiod/src/line_request.rs impl Request { · line 43
Quoted 6 lines
bindings/rust/libgpiod/src/line_request.rs +6
-0+// 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
Quoted 6 lines
Other than that the series looks good to me.
Reviewed-by Kent Gibson <[email protected]>Cheers,
Kent.
Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
Quoted 49 lines · Bartosz Golaszewski
Quoted 38 lines · Kent Gibson
Quoted 36 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a wrapper around
gpiod_line_request_get_chip_name()for Rust bindings and add a test-case.Signed-off-by Bartosz Golaszewski <[email protected]>--- bindings/rust/libgpiod/src/line_request.rs | 16 ++++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++ 2 files changed, 30 insertions(+)bindings/rust/libgpiod/src/line_request.rs +10
-0@@ -2,6 +2,7 @@// SPDX-FileCopyrightText: 2022 Linaro Ltd. // SPDX-FileCopyrightText: 2022 Viresh Kumar <[email protected]> +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 Quoted 9 lines
bindings/rust/libgpiod/src/line_request.rs impl Request { · line 43
Quoted 6 lines
bindings/rust/libgpiod/src/line_request.rs +6
-0+// 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.
Quoted 2 lines
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.
Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
Quoted 69 lines · Kent Gibson
Quoted 49 lines · Bartosz Golaszewski
Quoted 38 lines · Kent Gibson
Quoted 36 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
Provide a wrapper around
gpiod_line_request_get_chip_name()for Rust bindings and add a test-case.Signed-off-by Bartosz Golaszewski <[email protected]>--- bindings/rust/libgpiod/src/line_request.rs | 16 ++++++++++++++++ bindings/rust/libgpiod/tests/line_request.rs | 14 ++++++++++++++ 2 files changed, 30 insertions(+)bindings/rust/libgpiod/src/line_request.rs +10
-0@@ -2,6 +2,7 @@// SPDX-FileCopyrightText: 2022 Linaro Ltd. // SPDX-FileCopyrightText: 2022 Viresh Kumar <[email protected]> +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 Quoted 9 lines
bindings/rust/libgpiod/src/line_request.rs impl Request { · line 43
Quoted 6 lines
bindings/rust/libgpiod/src/line_request.rs +6
-0+// 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.
Quoted 2 lines
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) }
Quoted 6 lines
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.
I guess it does work. This looks less obvious to me though after all, so I left the previous version. I removed the last part of the SAFETY comment though as per your suggestion.
Thanks
Bart