treewide: unify gpiod_line_config/request_get_offsets() functions
Subsystems (2)
PATCH v4 1/1 treewide: unify gpiod_line_config/request_get_offsets() functions
From: Bartosz Golaszewski <[email protected]>
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.
---
v2 -> v3:
- use a custom type for the offset -> Settings mapping (defined as an IntMap<line::Settings>)
- add missing '// SAFETY' comments
- apply cargo fmt
v3 -> v4:
- apply missing cargo fmt hints (by using rustfmt)
bindings/cxx/line-config.cpp | 18 ++--- bindings/cxx/line-request.cpp | 6 +- bindings/python/gpiod/ext/request.c | 8 +-- bindings/rust/libgpiod/src/lib.rs | 3 + bindings/rust/libgpiod/src/line_config.rs | 67 +++++++----------- bindings/rust/libgpiod/src/line_request.rs | 11 ++- bindings/rust/libgpiod/tests/line_config.rs | 24 ++----- configure.ac | 1 + include/gpiod.h | 62 +++++++++++------ lib/line-config.c | 39 +++++------ lib/line-request.c | 23 ++++--- tests/tests-line-config.c | 76 +++++++++++++++------ tests/tests-line-request.c | 49 +++++++++++-- 13 files changed, 232 insertions(+), 155 deletions(-)
bindings/cxx/line-config.cpp +7 -11
@@ -102,31 +102,27 @@ GPIOD_CXX_API line_config& line_config::add_line_settings(const line::offsets& o GPIOD_CXX_API ::std::map<line::offset, line_settings> line_config::get_line_settings() const { + ::std::size_t num_offsets = ::gpiod_line_config_get_num_configured_offsets( + this->_m_priv->config.get()); ::std::map<line::offset, line_settings> settings_map; - ::std::size_t num_offsets; - unsigned int *offsets_ptr; - int ret; - - ret = ::gpiod_line_config_get_offsets(this->_m_priv->config.get(), - &num_offsets, &offsets_ptr); - if (ret) - throw_from_errno("unable to retrieve line offsets"); + ::std::vector<unsigned int> offsets(num_offsets); if (num_offsets == 0) return settings_map; - ::std::unique_ptr<unsigned int, malloc_deleter> offsets(offsets_ptr); + ::gpiod_line_config_get_configured_offsets(this->_m_priv->config.get(), + offsets.data(), num_offsets); for (size_t i = 0; i < num_offsets; i++) { line_settings settings; settings._m_priv->settings.reset(::gpiod_line_config_get_line_settings( this->_m_priv->config.get(), - offsets.get()[i])); + offsets[i])); if (!settings._m_priv->settings) throw_from_errno("unable to retrieve line settings"); - settings_map[offsets.get()[i]] = ::std::move(settings); + settings_map[offsets[i]] = ::std::move(settings); } return settings_map;
bindings/cxx/line-request.cpp +3 -3
@@ -18,7 +18,7 @@ void line_request::impl::throw_if_released() const void line_request::impl::set_request_ptr(line_request_ptr& ptr) { this->request = ::std::move(ptr); - this->offset_buf.resize(::gpiod_line_request_get_num_lines(this->request.get())); + this->offset_buf.resize(::gpiod_line_request_get_num_requested_lines(this->request.get())); } void line_request::impl::fill_offset_buf(const line::offsets& offsets)
@@ -67,7 +67,7 @@ GPIOD_CXX_API ::std::size_t line_request::num_lines() const { this->_m_priv->throw_if_released(); - return ::gpiod_line_request_get_num_lines(this->_m_priv->request.get()); + return ::gpiod_line_request_get_num_requested_lines(this->_m_priv->request.get()); } GPIOD_CXX_API line::offsets line_request::offsets() const
@@ -78,7 +78,7 @@ GPIOD_CXX_API line::offsets line_request::offsets() const ::std::vector<unsigned int> buf(num_lines); line::offsets offsets(num_lines); - ::gpiod_line_request_get_offsets(this->_m_priv->request.get(), buf.data()); + ::gpiod_line_request_get_requested_offsets(this->_m_priv->request.get(), buf.data(), buf.size()); for (unsigned int i = 0; i < num_lines; i++) offsets[i] = buf[i];
bindings/python/gpiod/ext/request.c +4 -4
@@ -41,7 +41,7 @@ static PyObject * request_num_lines(request_object *self, void *Py_UNUSED(ignored)) {return PyLong_FromUnsignedLong ( -gpiod_line_request_get_num_lines (self->request)); +gpiod_line_request_get_num_requested_lines (self->request)); }static PyObject *request_offsets (request_object *self,void *Py_UNUSED (ignored ))
@@ -51,13 +51,13 @@ static PyObject *request_offsets(request_object *self, void *Py_UNUSED(ignored))size_t num_lines, i;int ret; - num_lines =gpiod_line_request_get_num_lines (self->request); + num_lines =gpiod_line_request_get_num_requested_lines (self->request); offsets =PyMem_Calloc (num_lines,sizeof (unsigned int ));if (!offsets)return PyErr_NoMemory (); -gpiod_line_request_get_offsets (self->request, offsets); +gpiod_line_request_get_requested_offsets (self->request, offsets, num_lines); lines =PyList_New (num_lines);if (!lines) {
@@ -365,7 +365,7 @@ PyObject *Py_gpiod_MakeRequestObject(struct gpiod_line_request *request,unsigned int *offsets;size_t num_lines; - num_lines =gpiod_line_request_get_num_lines (request); + num_lines =gpiod_line_request_get_num_requested_lines (request); req_obj =PyObject_New (request_object, &request_type);if (!req_obj)
bindings/rust/libgpiod/src/lib.rs +3 -0
@@ -178,6 +178,9 @@ pub mod line {/// Maps offset to Value. pub type ValueMap =IntMap <Value >; +/// Maps offsets to Settings +pub type SettingsMap =IntMap <Settings >; +impl Value {pub fn new (val: gpiod::gpiod_line_value ) ->Result <Self > { Ok(match val {
bindings/rust/libgpiod/src/line_config.rs +26 -41
89 lines not on this page · raw message
bindings/rust/libgpiod/src/line_request.rs +9 -2
@@ -28,7 +28,7 @@ impl Request {/// Get the number of lines in the request. pub fn num_lines (&self) ->usize {// SAFETY: `gpiod_line_request` is guaranteed to be valid here. -unsafe { gpiod::gpiod_line_request_get_num_lines (self.request)as usize } +unsafe { gpiod::gpiod_line_request_get_num_requested_lines (self.request)as usize } }/// Get the offsets of lines in the request.
@@ -36,7 +36,14 @@ impl Request {let mut offsets =vec! [0 ; self.num_lines ()as usize ];// SAFETY: `gpiod_line_request` is guaranteed to be valid here. -unsafe { gpiod::gpiod_line_request_get_offsets (self.request, offsets.as_mut_ptr ()) }; +let num_offsets =unsafe { + gpiod::gpiod_line_request_get_requested_offsets ( + self.request, + offsets.as_mut_ptr (), + self.num_lines ()as u64 , + ) + }; + offsets.shrink_to (num_offsetsas usize ); offsets }
bindings/rust/libgpiod/tests/line_config.rs +4 -20
@@ -37,8 +37,10 @@ mod line_config { lconfig.add_line_settings (&[0 ,1 ,2 ], lsettings1).unwrap (); lconfig.add_line_settings (&[4 ,5 ], lsettings2).unwrap (); +let settings_map = lconfig.line_settings ().unwrap (); +// Retrieve settings -let lsettings = lconfig.line_settings (1 ).unwrap (); +let lsettings = settings_map.get (1 ).unwrap ();assert_eq! ( lsettings.prop (SettingKind ::Direction ).unwrap (),SettingVal ::Direction (Direction ::Input )
@@ -56,7 +58,7 @@ mod line_config {SettingVal ::Drive (Drive ::PushPull ) ); -let lsettings = lconfig.line_settings (5 ).unwrap (); +let lsettings = settings_map.get (5 ).unwrap ();assert_eq! ( lsettings.prop (SettingKind ::Direction ).unwrap (),SettingVal ::Direction (Direction ::Output )
@@ -74,22 +76,4 @@ mod line_config {SettingVal ::OutputValue (Value ::Active ) ); } - -#[test] -fn offsets () { -let mut lsettings1 = line::Settings ::new ().unwrap (); - lsettings1.set_direction (Direction ::Input ).unwrap (); - -let mut lsettings2 = line::Settings ::new ().unwrap (); - lsettings2.set_event_clock (EventClock ::Realtime ).unwrap (); - -// Add settings for multiple lines -let lconfig = line::Config ::new ().unwrap (); - lconfig.add_line_settings (&[0 ,1 ,2 ], lsettings1).unwrap (); - lconfig.add_line_settings (&[4 ,5 ], lsettings2).unwrap (); - -// Verify offsets -let offsets = lconfig.offsets ().unwrap (); -assert_eq! (offsets, [0 ,1 ,2 ,4 ,5 ]); - } }
configure.ac +1 -0
@@ -88,6 +88,7 @@ AC_CHECK_HEADERS([dirent.h], [], [HEADER_NOT_FOUND_LIB([dirent.h])]) AC_CHECK_HEADERS([poll.h], [], [HEADER_NOT_FOUND_LIB([poll.h])]) AC_CHECK_HEADERS([sys/sysmacros.h], [], [HEADER_NOT_FOUND_LIB([sys/sysmacros.h])]) AC_CHECK_HEADERS([sys/ioctl.h], [], [HEADER_NOT_FOUND_LIB([sys/ioctl.h])]) +AC_CHECK_HEADERS([sys/param.h], [], [HEADER_NOT_FOUND_LIB([sys/param.h])]) AC_CHECK_HEADERS([sys/stat.h], [], [HEADER_NOT_FOUND_LIB([sys/stat.h])]) AC_CHECK_HEADERS([sys/types.h], [], [HEADER_NOT_FOUND_LIB([sys/types.h])]) AC_CHECK_HEADERS([linux/const.h], [], [HEADER_NOT_FOUND_LIB([linux/const.h])])
include/gpiod.h +40 -22
97 lines not on this page · raw message
lib/line-config.c +18 -21
@@ -6,6 +6,7 @@#include <gpiod.h> #include <stdlib.h> #include <string.h> +#include <sys/param.h> #include "internal.h"
@@ -152,36 +153,32 @@ gpiod_line_config_get_line_settings(struct gpiod_line_config *config,return NULL ; } -GPIOD_API int gpiod_line_config_get_offsets(struct gpiod_line_config *config, -size_t *num_offsets, -unsigned int **offsets) +GPIOD_API size_t +gpiod_line_config_get_num_configured_offsets (struct gpiod_line_config *config) { -unsigned int *offs; -size_t i; -assert (config); -if (!num_offsets || !offsets) { - errno =EINVAL ; -return -1 ; - } +return config->num_configs; +} - *num_offsets = config->num_configs; - *offsets =NULL ; +GPIOD_API size_t +gpiod_line_config_get_configured_offsets (struct gpiod_line_config *config, +unsigned int *offsets, +size_t max_offsets) +{ +size_t num_offsets, i; -if (!config->num_configs) -return 0 ; +assert (config); - offs =calloc (config->num_configs,sizeof (unsigned int )); -if (!offs) -return -1 ; +if (!offsets || !max_offsets || !config->num_configs) +return 0 ; -for (i =0 ; i < config->num_configs; i++) - offs[i] = config->line_configs[i].offset; + num_offsets =MIN (config->num_configs, max_offsets); - *offsets = offs; +for (i =0 ; i < num_offsets; i++) + offsets[i] = config->line_configs[i].offset; -return 0 ; +return num_offsets; } static void set_offsets(struct gpiod_line_config *config,
lib/line-request.c +15 -8
@@ -7,6 +7,7 @@#include <stdlib.h> #include <string.h> #include <sys/ioctl.h> +#include <sys/param.h> #include <unistd.h> #include "internal.h"
@@ -45,24 +46,30 @@ GPIOD_API void gpiod_line_request_release(struct gpiod_line_request *request) } GPIOD_API size_t -gpiod_line_request_get_num_lines (struct gpiod_line_request *request) +gpiod_line_request_get_num_requested_lines (struct gpiod_line_request *request) {assert (request);return request->num_lines; } -GPIOD_API void -gpiod_line_request_get_offsets (struct gpiod_line_request *request, -unsigned int *offsets) +GPIOD_API size_t +gpiod_line_request_get_requested_offsets (struct gpiod_line_request *request, +unsigned int *offsets, +size_t max_offsets) { +size_t num_offsets; +assert (request); -if (!offsets) -return ; +if (!offsets || !max_offsets) +return 0 ; + + num_offsets =MIN (request->num_lines, max_offsets); + +memcpy (offsets, request->offsets,sizeof (*offsets) * num_offsets); -memcpy (offsets, request->offsets, -sizeof (*offsets) * request->num_lines); +return num_offsets; } GPIOD_API enum gpiod_line_value
tests/tests-line-config.c +57 -19
106 lines not on this page · raw message
tests/tests-line-request.c +45 -4
66 lines not on this page · raw message
Signature · 1 line
2.37.2
Quoted 31 lines · Bartosz Golaszewski
From: Bartosz Golaszewski <[email protected]>
We have two functions in the C API that allow users to retrieve a list of offsets from objects:
gpiod_line_request_get_offsets()andgpiod_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-libgpiodfree()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 <[email protected]>---
v2 -> v3:
- use a custom type for the offset -> Settings mapping (defined as an IntMap<line::Settings>)
- add missing '// SAFETY' comments
- apply cargo fmt
Quoted 2 lines
v3 -> v4:
- apply missing cargo fmt hints (by using rustfmt)
Signature · 1 line
viresh