* [libgpiod][PATCH v2 1/5] core: provide gpiod_line_request_get_chip_name()
2023-07-20 14:47 [libgpiod][PATCH v2 0/5] core: provide information about the parent chip in line requests Bartosz Golaszewski
@ 2023-07-20 14:47 ` Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 2/5] tests: add a test-case for gpiod_line_request_get_chip_name() Bartosz Golaszewski
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-07-20 14:47 UTC (permalink / raw)
To: Kent Gibson, Linus Walleij, Andy Shevchenko, Viresh Kumar,
Erik Schilling
Cc: linux-gpio, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
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.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
include/gpiod.h | 9 +++++++++
lib/chip.c | 7 ++++++-
lib/internal.h | 3 ++-
lib/line-request.c | 20 +++++++++++++++++++-
4 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/include/gpiod.h b/include/gpiod.h
index 3c13783..71ae798 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -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.
diff --git a/lib/chip.c b/lib/chip.c
index 7d4d21e..7c05e53 100644
--- a/lib/chip.c
+++ b/lib/chip.c
@@ -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;
diff --git a/lib/internal.h b/lib/internal.h
index ef9b17e..61d7aec 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -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);
diff --git a/lib/line-request.c b/lib/line-request.c
index e536355..e867d91 100644
--- a/lib/line-request.c
+++ b/lib/line-request.c
@@ -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_t
gpiod_line_request_get_num_requested_lines(struct gpiod_line_request *request)
{
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [libgpiod][PATCH v2 2/5] tests: add a test-case for gpiod_line_request_get_chip_name()
2023-07-20 14:47 [libgpiod][PATCH v2 0/5] core: provide information about the parent chip in line requests Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 1/5] core: provide gpiod_line_request_get_chip_name() Bartosz Golaszewski
@ 2023-07-20 14:47 ` Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 3/5] bindings: cxx: provide line_request::chip_name() Bartosz Golaszewski
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-07-20 14:47 UTC (permalink / raw)
To: Kent Gibson, Linus Walleij, Andy Shevchenko, Viresh Kumar,
Erik Schilling
Cc: linux-gpio, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Add a test case for the chip name getter on line-request objects.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
tests/tests-line-request.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tests/tests-line-request.c b/tests/tests-line-request.c
index 0b985e2..1a58275 100644
--- a/tests/tests-line-request.c
+++ b/tests/tests-line-request.c
@@ -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));
+}
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [libgpiod][PATCH v2 3/5] bindings: cxx: provide line_request::chip_name()
2023-07-20 14:47 [libgpiod][PATCH v2 0/5] core: provide information about the parent chip in line requests Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 1/5] core: provide gpiod_line_request_get_chip_name() Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 2/5] tests: add a test-case for gpiod_line_request_get_chip_name() Bartosz Golaszewski
@ 2023-07-20 14:47 ` Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 4/5] bindings: python: provide the chip_name property in line_request Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name() Bartosz Golaszewski
4 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-07-20 14:47 UTC (permalink / raw)
To: Kent Gibson, Linus Walleij, Andy Shevchenko, Viresh Kumar,
Erik Schilling
Cc: linux-gpio, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Provide a wrapper around gpiod_line_request_get_chip_name() for C++
bindings and update the tests.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
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(-)
diff --git a/bindings/cxx/gpiodcxx/line-request.hpp b/bindings/cxx/gpiodcxx/line-request.hpp
index c1e1520..8c1b474 100644
--- a/bindings/cxx/gpiodcxx/line-request.hpp
+++ b/bindings/cxx/gpiodcxx/line-request.hpp
@@ -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.
diff --git a/bindings/cxx/line-request.cpp b/bindings/cxx/line-request.cpp
index b0723c3..e8e0b96 100644
--- a/bindings/cxx/line-request.cpp
+++ b/bindings/cxx/line-request.cpp
@@ -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() <<
")";
diff --git a/bindings/cxx/tests/tests-line-request.cpp b/bindings/cxx/tests/tests-line-request.cpp
index d1a56ae..9632ae0 100644
--- a/bindings/cxx/tests/tests-line-request.cpp
+++ b/bindings/cxx/tests/tests-line-request.cpp
@@ -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")
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [libgpiod][PATCH v2 4/5] bindings: python: provide the chip_name property in line_request
2023-07-20 14:47 [libgpiod][PATCH v2 0/5] core: provide information about the parent chip in line requests Bartosz Golaszewski
` (2 preceding siblings ...)
2023-07-20 14:47 ` [libgpiod][PATCH v2 3/5] bindings: cxx: provide line_request::chip_name() Bartosz Golaszewski
@ 2023-07-20 14:47 ` Bartosz Golaszewski
2023-07-20 14:47 ` [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name() Bartosz Golaszewski
4 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-07-20 14:47 UTC (permalink / raw)
To: Kent Gibson, Linus Walleij, Andy Shevchenko, Viresh Kumar,
Erik Schilling
Cc: linux-gpio, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Provide a wrapper around gpiod_line_request_get_chip_name() for Python
bindings and update the tests.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
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(-)
diff --git a/bindings/python/gpiod/chip.py b/bindings/python/gpiod/chip.py
index da93370..b3d8e61 100644
--- a/bindings/python/gpiod/chip.py
+++ b/bindings/python/gpiod/chip.py
@@ -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
diff --git a/bindings/python/gpiod/ext/request.c b/bindings/python/gpiod/ext/request.c
index a32ff8f..5db69fe 100644
--- a/bindings/python/gpiod/ext/request.c
+++ b/bindings/python/gpiod/ext/request.c
@@ -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,
diff --git a/bindings/python/gpiod/line_request.py b/bindings/python/gpiod/line_request.py
index 096bf18..cde298f 100644
--- a/bindings/python/gpiod/line_request.py
+++ b/bindings/python/gpiod/line_request.py
@@ -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:
"""
diff --git a/bindings/python/tests/tests_line_request.py b/bindings/python/tests/tests_line_request.py
index aa84b9a..f99b93d 100644
--- a/bindings/python/tests/tests_line_request.py
+++ b/bindings/python/tests/tests_line_request.py
@@ -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})
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-20 14:47 [libgpiod][PATCH v2 0/5] core: provide information about the parent chip in line requests Bartosz Golaszewski
` (3 preceding siblings ...)
2023-07-20 14:47 ` [libgpiod][PATCH v2 4/5] bindings: python: provide the chip_name property in line_request Bartosz Golaszewski
@ 2023-07-20 14:47 ` Bartosz Golaszewski
2023-07-20 14:53 ` Erik Schilling
` (3 more replies)
4 siblings, 4 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-07-20 14:47 UTC (permalink / raw)
To: Kent Gibson, Linus Walleij, Andy Shevchenko, Viresh Kumar,
Erik Schilling
Cc: linux-gpio, Bartosz Golaszewski
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)
+ }
+
/// Get the number of lines in the request.
pub fn num_lines(&self) -> usize {
// SAFETY: `gpiod_line_request` is guaranteed to be valid here.
diff --git a/bindings/rust/libgpiod/tests/line_request.rs b/bindings/rust/libgpiod/tests/line_request.rs
index d49874f..9af5226 100644
--- a/bindings/rust/libgpiod/tests/line_request.rs
+++ b/bindings/rust/libgpiod/tests/line_request.rs
@@ -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;
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-20 14:47 ` [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name() Bartosz Golaszewski
@ 2023-07-20 14:53 ` Erik Schilling
2023-07-21 3:15 ` Kent Gibson
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Erik Schilling @ 2023-07-20 14:53 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: linux-gpio, Bartosz Golaszewski, Kent Gibson, Linus Walleij,
Andy Shevchenko, Viresh Kumar
On Thu Jul 20, 2023 at 4:47 PM CEST, 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(+)
Reviewed-by: Erik Schilling <erik.schilling@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-20 14:47 ` [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name() Bartosz Golaszewski
2023-07-20 14:53 ` Erik Schilling
@ 2023-07-21 3:15 ` Kent Gibson
2023-07-21 18:35 ` Bartosz Golaszewski
2023-07-21 3:27 ` Kent Gibson
2023-07-21 6:55 ` Viresh Kumar
3 siblings, 1 reply; 13+ messages in thread
From: Kent Gibson @ 2023-07-21 3:15 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Andy Shevchenko, Viresh Kumar, Erik Schilling,
linux-gpio, Bartosz Golaszewski
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.
Other than that the series looks good to me.
Reviewed-by: Kent Gibson <warthog618@gmail.com>
Cheers,
Kent.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-21 3:15 ` Kent Gibson
@ 2023-07-21 18:35 ` Bartosz Golaszewski
2023-07-22 2:07 ` Kent Gibson
0 siblings, 1 reply; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-07-21 18:35 UTC (permalink / raw)
To: Kent Gibson
Cc: Linus Walleij, Andy Shevchenko, Viresh Kumar, Erik Schilling,
linux-gpio, Bartosz Golaszewski
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. :)
Bart
> Other than that the series looks good to me.
>
> Reviewed-by: Kent Gibson <warthog618@gmail.com>
>
> Cheers,
> Kent.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-21 18:35 ` Bartosz Golaszewski
@ 2023-07-22 2:07 ` Kent Gibson
2023-07-22 12:30 ` Bartosz Golaszewski
0 siblings, 1 reply; 13+ messages in thread
From: Kent Gibson @ 2023-07-22 2:07 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Andy Shevchenko, Viresh Kumar, Erik Schilling,
linux-gpio, Bartosz Golaszewski
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-22 2:07 ` Kent Gibson
@ 2023-07-22 12:30 ` Bartosz Golaszewski
0 siblings, 0 replies; 13+ messages in thread
From: Bartosz Golaszewski @ 2023-07-22 12:30 UTC (permalink / raw)
To: Kent Gibson
Cc: Linus Walleij, Andy Shevchenko, Viresh Kumar, Erik Schilling,
linux-gpio, Bartosz Golaszewski
On Sat, Jul 22, 2023 at 4:07 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> 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.
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-20 14:47 ` [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name() Bartosz Golaszewski
2023-07-20 14:53 ` Erik Schilling
2023-07-21 3:15 ` Kent Gibson
@ 2023-07-21 3:27 ` Kent Gibson
2023-07-21 6:55 ` Viresh Kumar
3 siblings, 0 replies; 13+ messages in thread
From: Kent Gibson @ 2023-07-21 3:27 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Andy Shevchenko, Viresh Kumar, Erik Schilling,
linux-gpio, Bartosz Golaszewski
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.
>
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name()
2023-07-20 14:47 ` [libgpiod][PATCH v2 5/5] bindings: rust: provide LineRequest::chip_name() Bartosz Golaszewski
` (2 preceding siblings ...)
2023-07-21 3:27 ` Kent Gibson
@ 2023-07-21 6:55 ` Viresh Kumar
3 siblings, 0 replies; 13+ messages in thread
From: Viresh Kumar @ 2023-07-21 6:55 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Kent Gibson, Linus Walleij, Andy Shevchenko, Erik Schilling,
linux-gpio, Bartosz Golaszewski
On 20-07-23, 16:47, 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(+)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply [flat|nested] 13+ messages in thread