Skip to contentSkip to conversation

treewide: start shaving off cruft for v2.0

PATCH In review Bartosz Golaszewski 2 participants 22 messages Dec 10, 2020 – Dec 14, 2020

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover treewide: start shaving off cruft for v2.0

From: Bartosz Golaszewski <[email protected]>

The following series removes a lot of interfaces that were deemed overkill in libgpiod and the removal of which was suggested to me before proceeding with the new API.

This leaves a couple holes in the library but we'll follow them up with more improvements all over the tree. We'll create a new object called gpiod_request for dealing with line requests of arbitrary size. We'll probably remove the the bulk objects from bindings and eventually we'll switch to using the v2 kernel uAPI.

Andy - a note for you: I know you're always very thorough in your reviews but in this case let's consider this series preparing a construction zone for the new API. Please don't nitpick too much. :)

Bartosz Golaszewski (14):
bindings: cxx: check for error from gpiod_line_bulk_new()
build: drop the message about tests having been built successfully
core: export gpiod_is_gpiochip_device()
bulk: drop the limit on the max number of lines
core: drop line iterators
treewide: kill opening chips by label
API: move gpiod_line_get_chip() to line attributes section
core: kill gpiod_line_close_chip()
core: kill gpiod_line_get()
treewide: kill global line lookup
treewide: kill find_lines()
core: rework gpiod_chip_find_line()
build: add a configure switch for building examples
core: kill chip iterators

 bindings/cxx/Makefile.am                |   8 +-
 bindings/cxx/chip.cpp                   |  47 +--
 bindings/cxx/examples/Makefile.am       |   4 +-
 bindings/cxx/examples/gpiodetectcxx.cpp |  13 +-
 bindings/cxx/examples/gpiofindcxx.cpp   |  18 +-
 bindings/cxx/examples/gpioinfocxx.cpp   |  43 ++-
 bindings/cxx/gpiod.hpp                  | 147 +-------
 bindings/cxx/iter.cpp                   |  95 +----
 bindings/cxx/line.cpp                   |  15 -
 bindings/cxx/line_bulk.cpp              |  24 +-
 bindings/cxx/tests/tests-chip.cpp       |  62 ++--
 bindings/cxx/tests/tests-iter.cpp       |  37 --
 bindings/cxx/tests/tests-line.cpp       |  19 -
 bindings/python/Makefile.am             |  10 +-
 bindings/python/examples/gpiodetect.py  |  12 +-
 bindings/python/examples/gpiofind.py    |  15 +-
 bindings/python/examples/gpioinfo.py    |  33 +-
 bindings/python/gpiodmodule.c           | 473 +++++++-----------------
 bindings/python/tests/gpiod_py_test.py  | 100 +----
 configure.ac                            |  12 +
 include/gpiod.h                         | 258 ++-----------
 lib/Makefile.am                         |   2 +-
 lib/core.c                              |  12 +-
 lib/helpers.c                           | 176 +++------
 lib/iter.c                              | 171 ---------
 tests/Makefile.am                       |  13 -
 tests/gpiod-test.h                      |   4 -
 tests/tests-bulk.c                      |   9 -
 tests/tests-chip.c                      |  97 +----
 tests/tests-iter.c                      | 123 ------
 tests/tests-line.c                      |  52 ---
 tools/gpiodetect.c                      |  29 +-
 tools/gpiofind.c                        |  34 +-
 tools/gpioinfo.c                        |  41 +-
 tools/gpiomon.c                         |   5 +-
 tools/tools-common.c                    |  15 +
 tools/tools-common.h                    |   3 +
 37 files changed, 557 insertions(+), 1674 deletions(-)
 delete mode 100644 lib/iter.c
 delete mode 100644 tests/tests-iter.c
Signature · 1 line
2.29.1
#1 lore raw

PATCH 1/14 bindings: cxx: check for error from gpiod_line_bulk_new()

From: Bartosz Golaszewski <[email protected]>

We call gpiod_line_bulk_new() in C++ bindings but never check its return value. This function can fail so add a private method to line_bulk that calls it and throws an exception if it returns NULL.

---
 bindings/cxx/gpiod.hpp     |  1 +
 bindings/cxx/line_bulk.cpp | 22 ++++++++++++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)
bindings/cxx/gpiod.hpp +1 -0
index a4ef7c9..8c8e6c9 100644
@@ -869,6 +869,7 @@ private:
 
 	using line_bulk_ptr = ::std::unique_ptr<::gpiod_line_bulk, line_bulk_deleter>;
 
+	line_bulk_ptr make_line_bulk_ptr(void) const;
 	line_bulk_ptr to_line_bulk(void) const;
 
 	::std::vector<line> _m_bulk;
bindings/cxx/line_bulk.cpp +16 -6
index c24e91d..1de90eb 100644
@@ -256,8 +256,7 @@ line_bulk line_bulk::event_wait(const ::std::chrono::nanoseconds& timeout) const
 	this->throw_if_empty();
 	line::chip_guard lock_chip(this->_m_bulk.front());
 
-	auto ev_bulk = ::gpiod_line_bulk_new(this->size());
-	line_bulk_ptr ev_bulk_deleter(ev_bulk);
+	auto ev_bulk = this->make_line_bulk_ptr();
 	auto bulk = this->to_line_bulk();
 	::timespec ts;
 	line_bulk ret;
@@ -266,16 +265,16 @@ line_bulk line_bulk::event_wait(const ::std::chrono::nanoseconds& timeout) const
 	ts.tv_sec = timeout.count() / 1000000000ULL;
 	ts.tv_nsec = timeout.count() % 1000000000ULL;
 
-	rv = ::gpiod_line_event_wait_bulk(bulk.get(), ::std::addressof(ts), ev_bulk);
+	rv = ::gpiod_line_event_wait_bulk(bulk.get(), ::std::addressof(ts), ev_bulk.get());
 	if (rv < 0) {
 		throw ::std::system_error(errno, ::std::system_category(),
 					  "error polling for events");
 	} else if (rv > 0) {
 		auto chip = this->_m_bulk[0].get_chip();
-		auto num_lines = ::gpiod_line_bulk_num_lines(ev_bulk);
+		auto num_lines = ::gpiod_line_bulk_num_lines(ev_bulk.get());
 
 		for (unsigned int i = 0; i < num_lines; i++)
-			ret.append(line(::gpiod_line_bulk_get_line(ev_bulk, i), chip));
+			ret.append(line(::gpiod_line_bulk_get_line(ev_bulk.get(), i), chip));
 	}
 
 	return ret;
@@ -340,10 +339,21 @@ void line_bulk::throw_if_empty(void) const
 		throw ::std::logic_error("line_bulk not holding any GPIO lines");
 }
 
-line_bulk::line_bulk_ptr line_bulk::to_line_bulk(void) const
+line_bulk::line_bulk_ptr line_bulk::make_line_bulk_ptr(void) const
 {
 	line_bulk_ptr bulk(::gpiod_line_bulk_new(this->size()));
 
+	if (!bulk)
+		throw ::std::system_error(errno, ::std::system_category(),
+					  "unable to allocate new bulk object");
+
+	return bulk;
+}
+
+line_bulk::line_bulk_ptr line_bulk::to_line_bulk(void) const
+{
+	line_bulk_ptr bulk = this->make_line_bulk_ptr();
+
 	for (auto& it: this->_m_bulk)
 		::gpiod_line_bulk_add_line(bulk.get(), it._m_line);
 
Signature · 1 line
2.29.1
#2 lore raw

PATCH 2/14 build: drop the message about tests having been built successfully

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

This is a leftover from early times. We now have multiple different test suites and none prints such a message. Drop it.

---
 tests/Makefile.am | 12 ------------
 1 file changed, 12 deletions(-)
tests/Makefile.am +0 -12
index 5c7edb6..2d3b959 100644
@@ -28,15 +28,3 @@ gpiod_test_SOURCES =			\
 		tests-iter.c		\
 		tests-line.c		\
 		tests-misc.c
-
-all-local: gpiod-test
-	@echo " ********************************************************"
-	@echo " * Core tests have been built as tests/gpiod-test.      *"
-	@echo " *                                                      *"
-	@echo " * They require a recent linux kernel version and the   *"
-	@echo " * gpio-mockup module (must not be built-in).           *"
-	@echo " *                                                      *"
-	@echo " * Run the test executable with superuser privileges or *"
-	@echo " * make sure /dev/gpiochipX files are readable and      *"
-	@echo " * writable by normal users.                            *"
-	@echo " ********************************************************"
Signature · 1 line
2.29.1
#3 lore raw

PATCH 3/14 core: export gpiod_is_gpiochip_device()

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We'll be dropping chip iterators treewide. Instead we'll encourage scanning /dev for GPIO chip devices. Exporting this function allows users to easily check if given file represents a GPIO chip character device.

---
 include/gpiod.h    |  8 ++++++++
 lib/core.c         |  4 ++--
 tests/tests-chip.c | 15 +++++++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)
include/gpiod.h +8 -0
index 355a99a..742dfc2 100644
@@ -71,6 +71,14 @@ struct gpiod_line_bulk;
  * Functions and data structures dealing with GPIO chips.
  */
 
+/**
+ * @brief Check if the file pointed to by path is a GPIO chip character device.
+ * @param path Path to check.
+ * @return True if the file exists and is a GPIO chip character device or a
+ *         symbolic link to it.
+ */
+bool gpiod_is_gpiochip_device(const char *path) GPIOD_API;
+
 /**
  * @brief Open a gpiochip by path.
  * @param path Path to the gpiochip device file.
lib/core.c +2 -2
index bf52fa6..efba959 100644
@@ -177,7 +177,7 @@ void gpiod_line_bulk_foreach_line(struct gpiod_line_bulk *bulk,
 	     (index) < (bulk)->num_lines;				\
 	     (index)++, (line) = (bulk)->lines[(index)])
 
-static bool is_gpiochip_cdev(const char *path)
+bool gpiod_is_gpiochip_device(const char *path)
 {
 	char *name, *realname, *sysfsp, sysfsdev[16], devstr[16];
 	struct stat statbuf;
@@ -282,7 +282,7 @@ struct gpiod_chip *gpiod_chip_open(const char *path)
 	 * We were able to open the file but is it really a gpiochip character
 	 * device?
 	 */
-	if (!is_gpiochip_cdev(path))
+	if (!gpiod_is_gpiochip_device(path))
 		goto err_close_fd;
 
 	chip = malloc(sizeof(*chip));
tests/tests-chip.c +15 -0
index 289c458..2f19c49 100644
@@ -11,6 +11,21 @@
 
 #define GPIOD_TEST_GROUP "chip"
 
+GPIOD_TEST_CASE(is_gpiochip_good, 0, { 8 })
+{
+	g_assert_true(gpiod_is_gpiochip_device(gpiod_test_chip_path(0)));
+}
+
+GPIOD_TEST_CASE(is_gpiochip_bad, 0, { 8 })
+{
+	g_assert_false(gpiod_is_gpiochip_device("/dev/null"));
+}
+
+GPIOD_TEST_CASE(is_gpiochip_nonexistent, 0, { 8 })
+{
+	g_assert_false(gpiod_is_gpiochip_device("/dev/nonexistent_gpiochip"));
+}
+
 GPIOD_TEST_CASE(open_good, 0, { 8 })
 {
 	g_autoptr(gpiod_chip_struct) chip = NULL;
Signature · 1 line
2.29.1
#4 lore raw

PATCH 4/14 bulk: drop the limit on the max number of lines

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

The limit of 64 lines max per bulk object is wrong. We may want to retrieve all lines from a chip exporting more than 64. We'll be reducing the role of bulk objects soon so drop this limit now.

---
 bindings/cxx/line_bulk.cpp    |  2 +-
 bindings/python/gpiodmodule.c | 14 ++++++++------
 include/gpiod.h               |  7 -------
 lib/core.c                    |  8 +++++---
 tests/tests-bulk.c            |  9 ---------
 tools/gpiomon.c               |  5 ++++-
 6 files changed, 18 insertions(+), 27 deletions(-)
bindings/cxx/line_bulk.cpp +1 -1
index 1de90eb..6e88d21 100644
@@ -48,7 +48,7 @@ const ::std::map<::std::bitset<32>, int, bitset_cmp> reqflag_mapping = {
 
 } /* namespace */
 
-const unsigned int line_bulk::MAX_LINES = GPIOD_LINE_BULK_MAX_LINES;
+const unsigned int line_bulk::MAX_LINES = 64;
 
 line_bulk::line_bulk(const ::std::vector<line>& lines)
 	: _m_bulk()
bindings/python/gpiodmodule.c +8 -6
index b5e69a5..b9b5770 100644
@@ -8,6 +8,8 @@
 #include <Python.h>
 #include <gpiod.h>
 
+#define LINE_REQUEST_MAX_LINES 64
+
 typedef struct {
 	PyObject_HEAD
 	struct gpiod_chip *chip;
@@ -1138,7 +1140,7 @@ static int gpiod_LineBulk_init(gpiod_LineBulkObject *self,
 				"Argument must be a non-empty sequence");
 		return -1;
 	}
-	if (self->num_lines > GPIOD_LINE_BULK_MAX_LINES) {
+	if (self->num_lines > LINE_REQUEST_MAX_LINES) {
 		PyErr_SetString(PyExc_TypeError,
 				"Too many objects in the sequence");
 		return -1;
@@ -1334,7 +1336,7 @@ static PyObject *gpiod_LineBulk_request(gpiod_LineBulkObject *self,
 				  NULL };
 
 	int rv, type = gpiod_LINE_REQ_DIR_AS_IS, flags = 0,
-	    default_vals[GPIOD_LINE_BULK_MAX_LINES], val;
+	    default_vals[LINE_REQUEST_MAX_LINES], val;
 	PyObject *def_vals_obj = NULL, *iter, *next;
 	struct gpiod_line_request_config conf;
 	struct gpiod_line_bulk *bulk;
@@ -1413,7 +1415,7 @@ PyDoc_STRVAR(gpiod_LineBulk_get_values_doc,
 static PyObject *gpiod_LineBulk_get_values(gpiod_LineBulkObject *self,
 					   PyObject *Py_UNUSED(ignored))
 {
-	int rv, vals[GPIOD_LINE_BULK_MAX_LINES];
+	int rv, vals[LINE_REQUEST_MAX_LINES];
 	struct gpiod_line_bulk *bulk;
 	PyObject *val_list, *val;
 	Py_ssize_t i;
@@ -1506,7 +1508,7 @@ PyDoc_STRVAR(gpiod_LineBulk_set_values_doc,
 static PyObject *gpiod_LineBulk_set_values(gpiod_LineBulkObject *self,
 					   PyObject *args)
 {
-	int rv, vals[GPIOD_LINE_BULK_MAX_LINES];
+	int rv, vals[LINE_REQUEST_MAX_LINES];
 	struct gpiod_line_bulk *bulk;
 	PyObject *val_list;
 
@@ -1556,7 +1558,7 @@ PyDoc_STRVAR(gpiod_LineBulk_set_config_doc,
 static PyObject *gpiod_LineBulk_set_config(gpiod_LineBulkObject *self,
 					   PyObject *args)
 {
-	int rv, vals[GPIOD_LINE_BULK_MAX_LINES];
+	int rv, vals[LINE_REQUEST_MAX_LINES];
 	struct gpiod_line_bulk *bulk;
 	PyObject *val_list;
 	const int *valp;
@@ -1672,7 +1674,7 @@ static PyObject *gpiod_LineBulk_set_direction_output(
 				gpiod_LineBulkObject *self,
 				PyObject *args)
 {
-	int rv, vals[GPIOD_LINE_BULK_MAX_LINES];
+	int rv, vals[LINE_REQUEST_MAX_LINES];
 	struct gpiod_line_bulk *bulk;
 	PyObject *val_list;
 	const int *valp;
include/gpiod.h +0 -7
index 742dfc2..9ffb446 100644
@@ -225,17 +225,10 @@ gpiod_chip_find_lines(struct gpiod_chip *chip, const char **names) GPIOD_API;
  * on multiple lines at once.
  */
 
-/**
- * @brief Maximum number of GPIO lines that can be requested at once or stored
- *        in a line bulk object at the same time.
- */
-#define GPIOD_LINE_BULK_MAX_LINES	64
-
 /**
  * @brief Allocate and initialize a new line bulk object.
  * @param max_lines Maximum number of lines this object can hold.
  * @return New line bulk object or NULL on error.
- * @note max_lines must not exceed ::GPIOD_LINE_BULK_MAX_LINES.
  */
 struct gpiod_line_bulk *gpiod_line_bulk_new(unsigned int max_lines) GPIOD_API;
 
lib/core.c +5 -3
index efba959..d96e6cf 100644
@@ -22,6 +22,8 @@
 #include <sys/types.h>
 #include <unistd.h>
 
+#define LINE_REQUEST_MAX_LINES	64
+
 enum {
 	LINE_FREE = 0,
 	LINE_REQUESTED_VALUES,
@@ -94,7 +96,7 @@ struct gpiod_line_bulk *gpiod_line_bulk_new(unsigned int max_lines)
 	struct gpiod_line_bulk *bulk;
 	size_t size;
 
-	if (max_lines < 1 || max_lines > GPIOD_LINE_BULK_MAX_LINES) {
+	if (max_lines == 0) {
 		errno = EINVAL;
 		return NULL;
 	}
@@ -1066,7 +1068,7 @@ int gpiod_line_set_flags(struct gpiod_line *line, int flags)
 int gpiod_line_set_flags_bulk(struct gpiod_line_bulk *bulk, int flags)
 {
 	struct gpiod_line *line;
-	int values[GPIOD_LINE_BULK_MAX_LINES];
+	int values[LINE_REQUEST_MAX_LINES];
 	unsigned int i;
 	int direction;
 
@@ -1129,7 +1131,7 @@ int gpiod_line_event_wait_bulk(struct gpiod_line_bulk *bulk,
 			       const struct timespec *timeout,
 			       struct gpiod_line_bulk *event_bulk)
 {
-	struct pollfd fds[GPIOD_LINE_BULK_MAX_LINES];
+	struct pollfd fds[LINE_REQUEST_MAX_LINES];
 	unsigned int off, num_lines;
 	struct gpiod_line *line;
 	int rv;
tests/tests-bulk.c +0 -9
index e2520fc..22cae84 100644
@@ -20,15 +20,6 @@ GPIOD_TEST_CASE(alloc_zero_lines, 0, { 1 })
 	g_assert_cmpint(errno, ==, EINVAL);
 }
 
-GPIOD_TEST_CASE(alloc_too_many_lines, 0, { 1 })
-{
-	struct gpiod_line_bulk *bulk;
-
-	bulk = gpiod_line_bulk_new(GPIOD_LINE_BULK_MAX_LINES + 1);
-	g_assert_null(bulk);
-	g_assert_cmpint(errno, ==, EINVAL);
-}
-
 GPIOD_TEST_CASE(add_too_many_lines, 0, { 8 })
 {
 	g_autoptr(gpiod_line_bulk_struct) bulk = NULL;
tools/gpiomon.c +4 -1
index 44fb431..c271913 100644
@@ -157,7 +157,7 @@ static void handle_signal(int signum UNUSED)
 
 int main(int argc, char **argv)
 {
-	unsigned int offsets[GPIOD_LINE_BULK_MAX_LINES], num_lines = 0, offset,
+	unsigned int offsets[64], num_lines = 0, offset,
 		     events_wanted = 0, events_done = 0, x;
 	bool watch_rising = false, watch_falling = false;
 	int flags = 0;
@@ -241,6 +241,9 @@ int main(int argc, char **argv)
 	if (argc < 2)
 		die("at least one GPIO line offset must be specified");
 
+	if (argc > 65)
+		die("too many offsets given");
+
 	for (i = 1; i < argc; i++) {
 		offset = strtoul(argv[i], &end, 10);
 		if (*end != '\0' || offset > INT_MAX)
Signature · 1 line
2.29.1
#5 lore raw

PATCH 5/14 core: drop line iterators

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Hand-crafted iterators don't make much sense in C and impose an additional layer of memory allocation and resource releasing. Remove the line iterators from the core C library.

We're leaving the iterators where they make sense: in C++ and Python bindings but we convert them to using other means of keeping track of lines.

---
 bindings/cxx/gpiod.hpp        |  1 -
 bindings/cxx/iter.cpp         | 28 +++++---------------
 bindings/python/gpiodmodule.c | 21 +++++----------
 include/gpiod.h               | 36 --------------------------
 lib/helpers.c                 | 32 ++++++++++-------------
 lib/iter.c                    | 48 -----------------------------------
 tests/gpiod-test.h            |  2 --
 tests/tests-iter.c            | 23 -----------------
 tools/gpioinfo.c              | 14 ++++------
 9 files changed, 33 insertions(+), 172 deletions(-)
bindings/cxx/gpiod.hpp +0 -1
index 8c8e6c9..0f1d9b2 100644
@@ -1083,7 +1083,6 @@ public:
 
 private:
 
-	::std::shared_ptr<::gpiod_line_iter> _m_iter;
 	line _m_current;
 };
 
bindings/cxx/iter.cpp +7 -21
index 7985910..15c3925 100644
@@ -17,23 +17,6 @@ void chip_iter_deleter(::gpiod_chip_iter* iter)
 	::gpiod_chip_iter_free_noclose(iter);
 }
 
-void line_iter_deleter(::gpiod_line_iter* iter)
-{
-	::gpiod_line_iter_free(iter);
-}
-
-::gpiod_line_iter* make_line_iter(::gpiod_chip* chip)
-{
-	::gpiod_line_iter* iter;
-
-	iter = ::gpiod_line_iter_new(chip);
-	if (!iter)
-		throw ::std::system_error(errno, ::std::system_category(),
-					  "error creating GPIO line iterator");
-
-	return iter;
-}
-
 } /* namespace */
 
 chip_iter make_chip_iter(void)
@@ -105,17 +88,20 @@ line_iter end(const line_iter&) noexcept
 }
 
 line_iter::line_iter(const chip& owner)
-	: _m_iter(make_line_iter(owner._m_chip.get()), line_iter_deleter),
-	  _m_current(line(::gpiod_line_iter_next(this->_m_iter.get()), owner))
+	: _m_current(owner.get_line(0))
 {
 
 }
 
 line_iter& line_iter::operator++(void)
 {
-	::gpiod_line* next = ::gpiod_line_iter_next(this->_m_iter.get());
+	unsigned int offset = this->_m_current.offset() + 1;
+	chip owner = this->_m_current.get_chip();
 
-	this->_m_current = next ? line(next, this->_m_current._m_owner) : line();
+	if (offset == owner.num_lines())
+		this->_m_current = line(); /* Last element */
+	else
+		this->_m_current = owner.get_line(offset);
 
 	return *this;
 }
bindings/python/gpiodmodule.c +7 -14
index b9b5770..11d1407 100644
@@ -41,7 +41,7 @@ typedef struct {
 
 typedef struct {
 	PyObject_HEAD
-	struct gpiod_line_iter *iter;
+	unsigned int offset;
 	gpiod_ChipObject *owner;
 } gpiod_LineIterObject;
 
@@ -2621,14 +2621,7 @@ static int gpiod_LineIter_init(gpiod_LineIterObject *self,
 	if (gpiod_ChipIsClosed(chip_obj))
 		return -1;
 
-	Py_BEGIN_ALLOW_THREADS;
-	self->iter = gpiod_line_iter_new(chip_obj->chip);
-	Py_END_ALLOW_THREADS;
-	if (!self->iter) {
-		PyErr_SetFromErrno(PyExc_OSError);
-		return -1;
-	}
-
+	self->offset = 0;
 	self->owner = chip_obj;
 	Py_INCREF(chip_obj);
 
@@ -2637,9 +2630,6 @@ static int gpiod_LineIter_init(gpiod_LineIterObject *self,
 
 static void gpiod_LineIter_dealloc(gpiod_LineIterObject *self)
 {
-	if (self->iter)
-		gpiod_line_iter_free(self->iter);
-
 	PyObject_Del(self);
 }
 
@@ -2647,10 +2637,13 @@ static gpiod_LineObject *gpiod_LineIter_next(gpiod_LineIterObject *self)
 {
 	struct gpiod_line *line;
 
-	line = gpiod_line_iter_next(self->iter);
-	if (!line)
+	if (self->offset == gpiod_chip_num_lines(self->owner->chip))
 		return NULL; /* Last element. */
 
+	line = gpiod_chip_get_line(self->owner->chip, self->offset++);
+	if (!line)
+		return (gpiod_LineObject *)PyErr_SetFromErrno(PyExc_OSError);
+
 	return gpiod_MakeLineObject(self->owner, line);
 }
 
include/gpiod.h +0 -36
index 9ffb446..b5965ed 100644
@@ -40,7 +40,6 @@ extern "C" {
 struct gpiod_chip;
 struct gpiod_line;
 struct gpiod_chip_iter;
-struct gpiod_line_iter;
 struct gpiod_line_bulk;
 
 /**
@@ -1202,41 +1201,6 @@ gpiod_chip_iter_next_noclose(struct gpiod_chip_iter *iter) GPIOD_API;
 	     (chip);							\
 	     (chip) = gpiod_chip_iter_next_noclose(iter))
 
-/**
- * @brief Create a new line iterator.
- * @param chip Active gpiochip handle over the lines of which we want
- *             to iterate.
- * @return New line iterator or NULL if an error occurred.
- */
-struct gpiod_line_iter *
-gpiod_line_iter_new(struct gpiod_chip *chip) GPIOD_API;
-
-/**
- * @brief Free all resources associated with a GPIO line iterator.
- * @param iter Line iterator object.
- */
-void gpiod_line_iter_free(struct gpiod_line_iter *iter) GPIOD_API;
-
-/**
- * @brief Get the next GPIO line handle.
- * @param iter The GPIO line iterator object.
- * @return Pointer to the next GPIO line handle or NULL if there are no more
- *         lines left.
- */
-struct gpiod_line *
-gpiod_line_iter_next(struct gpiod_line_iter *iter) GPIOD_API;
-
-/**
- * @brief Iterate over all GPIO lines of a single chip.
- * @param iter An initialized GPIO line iterator.
- * @param line Pointer to a GPIO line handle - on each iteration, the
- *             next GPIO line will be assigned to this argument.
- */
-#define gpiod_foreach_line(iter, line)					\
-	for ((line) = gpiod_line_iter_next(iter);			\
-	     (line);							\
-	     (line) = gpiod_line_iter_next(iter))
-
 /**
  * @}
  *
lib/helpers.c +14 -18
index a343f71..3b7428b 100644
@@ -124,24 +124,23 @@ gpiod_chip_get_lines(struct gpiod_chip *chip,
 
 struct gpiod_line_bulk *gpiod_chip_get_all_lines(struct gpiod_chip *chip)
 {
-	struct gpiod_line_iter *iter;
 	struct gpiod_line_bulk *bulk;
 	struct gpiod_line *line;
+	unsigned int offset;
 
 	bulk = gpiod_line_bulk_new(gpiod_chip_num_lines(chip));
 	if (!bulk)
 		return NULL;
 
-	iter = gpiod_line_iter_new(chip);
-	if (!iter) {
-		gpiod_line_bulk_free(bulk);
-		return NULL;
-	}
+	for (offset = 0; offset < gpiod_chip_num_lines(chip); offset++) {
+		line = gpiod_chip_get_line(chip, offset);
+		if (!line) {
+			gpiod_line_bulk_free(bulk);
+			return NULL;
+		}
 
-	gpiod_foreach_line(iter, line)
 		gpiod_line_bulk_add_line(bulk, line);
-
-	gpiod_line_iter_free(iter);
+	}
 
 	return bulk;
 }
@@ -149,24 +148,21 @@ struct gpiod_line_bulk *gpiod_chip_get_all_lines(struct gpiod_chip *chip)
 struct gpiod_line *
 gpiod_chip_find_line(struct gpiod_chip *chip, const char *name)
 {
-	struct gpiod_line_iter *iter;
 	struct gpiod_line *line;
+	unsigned int offset;
 	const char *tmp;
 
-	iter = gpiod_line_iter_new(chip);
-	if (!iter)
-		return NULL;
+	for (offset = 0; offset < gpiod_chip_num_lines(chip); offset++) {
+		line = gpiod_chip_get_line(chip, offset);
+		if (!line)
+			return NULL;
 
-	gpiod_foreach_line(iter, line) {
 		tmp = gpiod_line_name(line);
-		if (tmp && strcmp(tmp, name) == 0) {
-			gpiod_line_iter_free(iter);
+		if (tmp && strcmp(tmp, name) == 0)
 			return line;
-		}
 	}
 
 	errno = ENOENT;
-	gpiod_line_iter_free(iter);
 
 	return NULL;
 }
lib/iter.c +0 -48

57 lines not on this page · raw message

tests/gpiod-test.h +0 -2
index a43109a..df9f0c7 100644
@@ -26,12 +26,10 @@
 typedef struct gpiod_chip gpiod_chip_struct;
 typedef struct gpiod_line_bulk gpiod_line_bulk_struct;
 typedef struct gpiod_chip_iter gpiod_chip_iter_struct;
-typedef struct gpiod_line_iter gpiod_line_iter_struct;
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(gpiod_chip_struct, gpiod_chip_close);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(gpiod_line_bulk_struct, gpiod_line_bulk_free);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(gpiod_chip_iter_struct, gpiod_chip_iter_free);
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(gpiod_line_iter_struct, gpiod_line_iter_free);
 
 /* These are private definitions and should not be used directly. */
 typedef void (*_gpiod_test_func)(void);
tests/tests-iter.c +0 -23
index 8deee8e..163a820 100644
@@ -98,26 +98,3 @@ GPIOD_TEST_CASE(chip_iter_break, 0, { 8, 8, 8, 8, 8 })
 
 	g_assert_cmpuint(i, ==, 3);
 }
-
-GPIOD_TEST_CASE(line_iter, 0, { 8 })
-{
-	g_autoptr(gpiod_line_iter_struct) iter = NULL;
-	g_autoptr(gpiod_chip_struct) chip = NULL;
-	struct gpiod_line *line;
-	guint i = 0;
-
-	chip = gpiod_chip_open(gpiod_test_chip_path(0));
-	g_assert_nonnull(chip);
-	gpiod_test_return_if_failed();
-
-	iter = gpiod_line_iter_new(chip);
-	g_assert_nonnull(iter);
-	gpiod_test_return_if_failed();
-
-	gpiod_foreach_line(iter, line) {
-		g_assert_cmpuint(i, ==, gpiod_line_offset(line));
-		i++;
-	}
-
-	g_assert_cmpuint(i, ==, 8);
-}
tools/gpioinfo.c +5 -9
index 67be379..dd4a388 100644
@@ -116,22 +116,20 @@ static PRINTF(3, 4) void prinfo(bool *of,
 
 static void list_lines(struct gpiod_chip *chip)
 {
-	struct gpiod_line_iter *iter;
 	int direction, active_state;
 	const char *name, *consumer;
 	struct gpiod_line *line;
 	unsigned int i, offset;
 	bool flag_printed, of;
 
-	iter = gpiod_line_iter_new(chip);
-	if (!iter)
-		die_perror("error creating line iterator");
-
 	printf("%s - %u lines:\n",
 	       gpiod_chip_name(chip), gpiod_chip_num_lines(chip));
 
-	gpiod_foreach_line(iter, line) {
-		offset = gpiod_line_offset(line);
+	for (offset = 0; offset < gpiod_chip_num_lines(chip); offset++) {
+		line = gpiod_chip_get_line(chip, offset);
+		if (!line)
+			die_perror("unable to retrieve the line object from chip");
+
 		name = gpiod_line_name(line);
 		consumer = gpiod_line_consumer(line);
 		direction = gpiod_line_direction(line);
@@ -178,8 +176,6 @@ static void list_lines(struct gpiod_chip *chip)
 
 		printf("\n");
 	}
-
-	gpiod_line_iter_free(iter);
 }
 
 int main(int argc, char **argv)
Signature · 1 line
2.29.1
#6 lore raw

PATCH 6/14 treewide: kill opening chips by label

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Chip labels are not unique - opening chips by label may lead to errors or to ignoring chips with duplicate labels. Users can easily implement chip lookup by label themselves so remove this part from the core library and all bindings.

---
 bindings/cxx/chip.cpp                  |  6 -----
 bindings/cxx/gpiod.hpp                 |  2 --
 bindings/cxx/tests/tests-chip.cpp      | 20 ----------------
 bindings/python/gpiodmodule.c          | 17 ++++---------
 bindings/python/tests/gpiod_py_test.py |  8 -------
 include/gpiod.h                        | 14 ++---------
 lib/helpers.c                          | 33 ++++----------------------
 tests/tests-chip.c                     | 24 -------------------
 8 files changed, 10 insertions(+), 114 deletions(-)
bindings/cxx/chip.cpp +0 -6
index 1fc0723..dffa7a2 100644
@@ -30,11 +30,6 @@ namespace {
 	return ::gpiod_chip_open_by_name(device.c_str());
 }
 
-::gpiod_chip* open_by_label(const ::std::string& device)
-{
-	return ::gpiod_chip_open_by_label(device.c_str());
-}
-
 ::gpiod_chip* open_by_number(const ::std::string& device)
 {
 	return ::gpiod_chip_open_by_number(::std::stoul(device));
@@ -46,7 +41,6 @@ const ::std::map<int, open_func> open_funcs = {
 	{ chip::OPEN_LOOKUP,	open_lookup,	},
 	{ chip::OPEN_BY_PATH,	open_by_path,	},
 	{ chip::OPEN_BY_NAME,	open_by_name,	},
-	{ chip::OPEN_BY_LABEL,	open_by_label,	},
 	{ chip::OPEN_BY_NUMBER,	open_by_number,	},
 };
 
bindings/cxx/gpiod.hpp +0 -2
index 0f1d9b2..0d443b0 100644
@@ -190,8 +190,6 @@ public:
 		/**< Assume the string is a path to the GPIO chardev. */
 		OPEN_BY_NAME,
 		/**< Assume the string is the name of the chip */
-		OPEN_BY_LABEL,
-		/**< Assume the string is the label of the GPIO chip. */
 		OPEN_BY_NUMBER,
 		/**< Assume the string is the number of the GPIO chip. */
 	};
bindings/cxx/tests/tests-chip.cpp +0 -20
index 1c69872..4c9f113 100644
@@ -28,11 +28,6 @@ TEST_CASE("GPIO chip device can be opened in different modes", "[chip]")
 				::gpiod::chip::OPEN_BY_PATH));
 	}
 
-	SECTION("open by label")
-	{
-		REQUIRE_NOTHROW(::gpiod::chip("gpio-mockup-B", ::gpiod::chip::OPEN_BY_LABEL));
-	}
-
 	SECTION("open by number")
 	{
 		REQUIRE_NOTHROW(::gpiod::chip(::std::to_string(mockup::instance().chip_num(1)),
@@ -54,11 +49,6 @@ TEST_CASE("GPIO chip device can be opened with implicit lookup", "[chip]")
 		REQUIRE_NOTHROW(::gpiod::chip(mockup::instance().chip_path(1)));
 	}
 
-	SECTION("lookup by label")
-	{
-		REQUIRE_NOTHROW(::gpiod::chip("gpio-mockup-B"));
-	}
-
 	SECTION("lookup by number")
 	{
 		REQUIRE_NOTHROW(::gpiod::chip(::std::to_string(mockup::instance().chip_num(1))));
@@ -84,11 +74,6 @@ TEST_CASE("GPIO chip can be opened with the open() method in different modes", "
 					  ::gpiod::chip::OPEN_BY_PATH));
 	}
 
-	SECTION("open by label")
-	{
-		REQUIRE_NOTHROW(chip.open("gpio-mockup-B", ::gpiod::chip::OPEN_BY_LABEL));
-	}
-
 	SECTION("open by number")
 	{
 		REQUIRE_NOTHROW(chip.open(::std::to_string(mockup::instance().chip_num(1)),
@@ -126,11 +111,6 @@ TEST_CASE("GPIO chip can be opened with the open() method with implicit lookup",
 		REQUIRE_NOTHROW(chip.open(mockup::instance().chip_path(1)));
 	}
 
-	SECTION("lookup by label")
-	{
-		REQUIRE_NOTHROW(chip.open("gpio-mockup-B"));
-	}
-
 	SECTION("lookup by number")
 	{
 		REQUIRE_NOTHROW(chip.open(::std::to_string(mockup::instance().chip_num(1))));
bindings/python/gpiodmodule.c +4 -13
index 11d1407..17a58b1 100644
@@ -1961,7 +1961,6 @@ enum {
 	gpiod_OPEN_LOOKUP = 1,
 	gpiod_OPEN_BY_NAME,
 	gpiod_OPEN_BY_PATH,
-	gpiod_OPEN_BY_LABEL,
 	gpiod_OPEN_BY_NUMBER,
 };
 
@@ -1987,9 +1986,6 @@ static int gpiod_Chip_init(gpiod_ChipObject *self,
 	case gpiod_OPEN_BY_PATH:
 		self->chip = gpiod_chip_open(descr);
 		break;
-	case gpiod_OPEN_BY_LABEL:
-		self->chip = gpiod_chip_open_by_label(descr);
-		break;
 	case gpiod_OPEN_BY_NUMBER:
 		self->chip = gpiod_chip_open_by_number(atoi(descr));
 		break;
@@ -2503,10 +2499,10 @@ PyDoc_STRVAR(gpiod_ChipType_doc,
 "The Chip object's constructor takes a description string as argument the\n"
 "meaning of which depends on the second, optional parameter which defines\n"
 "the way the description string should be interpreted. The available\n"
-"options are: OPEN_BY_LABEL, OPEN_BY_NAME, OPEN_BY_NUMBER, OPEN_BY_PATH,\n"
-"and OPEN_LOOKUP. The last option means that libgpiod should open the chip\n"
-"based on the best guess what the path is. This is also the default if the\n"
-"second argument is missing.\n"
+"options are: OPEN_BY_NAME, OPEN_BY_NUMBER, OPEN_BY_PATH and OPEN_LOOKUP.\n"
+"The last option means that libgpiod should open the chip based on the best\n"
+"guess what the path is. This is also the default if the second argument is\n"
+"missing.\n"
 "\n"
 "Callers must close the chip by calling the close() method when it's no\n"
 "longer used.\n"
@@ -2786,11 +2782,6 @@ static gpiod_ConstDescr gpiod_ConstList[] = {
 		.name = "OPEN_BY_NAME",
 		.val = gpiod_OPEN_BY_NAME,
 	},
-	{
-		.typeobj = &gpiod_ChipType,
-		.name = "OPEN_BY_LABEL",
-		.val = gpiod_OPEN_BY_LABEL,
-	},
 	{
 		.typeobj = &gpiod_ChipType,
 		.name = "OPEN_BY_NUMBER",
bindings/python/tests/gpiod_py_test.py +0 -8
index cb15fdc..e4aaadc 100755
@@ -96,10 +96,6 @@ class ChipOpen(MockupTestCase):
                         gpiod.Chip.OPEN_BY_NUMBER) as chip:
             self.assertEqual(chip.name(), mockup.chip_name(1))
 
-    def test_open_chip_by_label(self):
-        with gpiod.Chip('gpio-mockup-B', gpiod.Chip.OPEN_BY_LABEL) as chip:
-            self.assertEqual(chip.name(), mockup.chip_name(1))
-
     def test_lookup_chip_by_name(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
             self.assertEqual(chip.name(), mockup.chip_name(1))
@@ -112,10 +108,6 @@ class ChipOpen(MockupTestCase):
         with gpiod.Chip('{}'.format(mockup.chip_num(1))) as chip:
             self.assertEqual(chip.name(), mockup.chip_name(1))
 
-    def test_lookup_chip_by_label(self):
-        with gpiod.Chip('gpio-mockup-B') as chip:
-            self.assertEqual(chip.name(), mockup.chip_name(1))
-
     def test_nonexistent_chip(self):
         with self.assertRaises(FileNotFoundError):
             chip = gpiod.Chip('nonexistent-chip')
include/gpiod.h +2 -12
index b5965ed..b28cc92 100644
@@ -103,24 +103,14 @@ struct gpiod_chip *gpiod_chip_open_by_name(const char *name) GPIOD_API;
  */
 struct gpiod_chip *gpiod_chip_open_by_number(unsigned int num) GPIOD_API;
 
-/**
- * @brief Open a gpiochip by label.
- * @param label Label of the gpiochip to open.
- * @return GPIO chip handle or NULL if the chip with given label was not found
- *         or an error occured.
- * @note If the chip cannot be found but no other error occurred, errno is set
- *       to ENOENT.
- */
-struct gpiod_chip *gpiod_chip_open_by_label(const char *label) GPIOD_API;
-
 /**
  * @brief Open a gpiochip based on the best guess what the path is.
  * @param descr String describing the gpiochip.
  * @return GPIO chip handle or NULL if an error occurred.
  *
  * This routine tries to figure out whether the user passed it the path to the
- * GPIO chip, its name, label or number as a string. Then it tries to open it
- * using one of the gpiod_chip_open** variants.
+ * GPIO chip, its name or number as a string. Then it tries to open it using
+ * one of the gpiod_chip_open** variants.
  */
 struct gpiod_chip *gpiod_chip_open_lookup(const char *descr) GPIOD_API;
 
lib/helpers.c +4 -29
index 3b7428b..3ec6c96 100644
@@ -56,28 +56,6 @@ struct gpiod_chip *gpiod_chip_open_by_number(unsigned int num)
 	return chip;
 }
 
-struct gpiod_chip *gpiod_chip_open_by_label(const char *label)
-{
-	struct gpiod_chip_iter *iter;
-	struct gpiod_chip *chip;
-
-	iter = gpiod_chip_iter_new();
-	if (!iter)
-		return NULL;
-
-	gpiod_foreach_chip(iter, chip) {
-		if (strcmp(label, gpiod_chip_label(chip)) == 0) {
-			gpiod_chip_iter_free_noclose(iter);
-			return chip;
-		}
-	}
-
-	errno = ENOENT;
-	gpiod_chip_iter_free(iter);
-
-	return NULL;
-}
-
 struct gpiod_chip *gpiod_chip_open_lookup(const char *descr)
 {
 	struct gpiod_chip *chip;
@@ -85,13 +63,10 @@ struct gpiod_chip *gpiod_chip_open_lookup(const char *descr)
 	if (isuint(descr)) {
 		chip = gpiod_chip_open_by_number(strtoul(descr, NULL, 10));
 	} else {
-		chip = gpiod_chip_open_by_label(descr);
-		if (!chip) {
-			if (strncmp(descr, "/dev/", 5))
-				chip = gpiod_chip_open_by_name(descr);
-			else
-				chip = gpiod_chip_open(descr);
-		}
+		if (strncmp(descr, "/dev/", 5))
+			chip = gpiod_chip_open_by_name(descr);
+		else
+			chip = gpiod_chip_open(descr);
 	}
 
 	return chip;
tests/tests-chip.c +0 -24
index 2f19c49..0c2948a 100644
@@ -68,28 +68,8 @@ GPIOD_TEST_CASE(open_by_number_good, 0, { 8 })
 	g_assert_nonnull(chip);
 }
 
-GPIOD_TEST_CASE(open_by_label_good, 0, { 4, 4, 4, 4, 4 })
-{
-	g_autoptr(gpiod_chip_struct) chip = NULL;
-
-	chip = gpiod_chip_open_by_label("gpio-mockup-D");
-	g_assert_nonnull(chip);
-	gpiod_test_return_if_failed();
-	g_assert_cmpstr(gpiod_chip_name(chip), ==, gpiod_test_chip_name(3));
-}
-
-GPIOD_TEST_CASE(open_by_label_bad, 0, { 4, 4, 4, 4, 4 })
-{
-	struct gpiod_chip *chip;
-
-	chip = gpiod_chip_open_by_label("nonexistent_gpio_chip");
-	g_assert_null(chip);
-	g_assert_cmpint(errno, ==, ENOENT);
-}
-
 GPIOD_TEST_CASE(open_lookup_good, 0, { 8, 8, 8})
 {
-	g_autoptr(gpiod_chip_struct) chip_by_label = NULL;
 	g_autoptr(gpiod_chip_struct) chip_by_name = NULL;
 	g_autoptr(gpiod_chip_struct) chip_by_path = NULL;
 	g_autoptr(gpiod_chip_struct) chip_by_num = NULL;
@@ -99,12 +79,10 @@ GPIOD_TEST_CASE(open_lookup_good, 0, { 8, 8, 8})
 	chip_by_name = gpiod_chip_open_lookup(gpiod_test_chip_name(1));
 	chip_by_path = gpiod_chip_open_lookup(gpiod_test_chip_path(1));
 	chip_by_num = gpiod_chip_open_lookup(chip_num_str);
-	chip_by_label = gpiod_chip_open_lookup("gpio-mockup-B");
 
 	g_assert_nonnull(chip_by_name);
 	g_assert_nonnull(chip_by_path);
 	g_assert_nonnull(chip_by_num);
-	g_assert_nonnull(chip_by_label);
 	gpiod_test_return_if_failed();
 
 	g_assert_cmpstr(gpiod_chip_name(chip_by_name),
@@ -113,8 +91,6 @@ GPIOD_TEST_CASE(open_lookup_good, 0, { 8, 8, 8})
 			==, gpiod_test_chip_name(1));
 	g_assert_cmpstr(gpiod_chip_name(chip_by_num),
 			==, gpiod_test_chip_name(1));
-	g_assert_cmpstr(gpiod_chip_name(chip_by_label),
-			==, gpiod_test_chip_name(1));
 }
 
 GPIOD_TEST_CASE(get_name, 0, { 8, 8, 8})
Signature · 1 line
2.29.1
#7 lore raw

PATCH 7/14 API: move gpiod_line_get_chip() to line attributes section

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

This routine should logically be part of line attributes. Move it out of the misc line section.

---
 include/gpiod.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
include/gpiod.h +7 -7
index b28cc92..c6e0b65 100644
@@ -431,6 +431,13 @@ bool gpiod_line_is_open_source(struct gpiod_line *line) GPIOD_API;
  */
 int gpiod_line_update(struct gpiod_line *line) GPIOD_API;
 
+/**
+ * @brief Get the handle to the GPIO chip controlling this line.
+ * @param line The GPIO line object.
+ * @return Pointer to the GPIO chip handle controlling this line.
+ */
+struct gpiod_chip *gpiod_line_get_chip(struct gpiod_line *line) GPIOD_API;
+
 /**
  * @}
  *
@@ -1093,13 +1100,6 @@ struct gpiod_line *gpiod_line_find(const char *name) GPIOD_API;
  */
 void gpiod_line_close_chip(struct gpiod_line *line) GPIOD_API;
 
-/**
- * @brief Get the handle to the GPIO chip controlling this line.
- * @param line The GPIO line object.
- * @return Pointer to the GPIO chip handle controlling this line.
- */
-struct gpiod_chip *gpiod_line_get_chip(struct gpiod_line *line) GPIOD_API;
-
 /**
  * @}
  *
Signature · 1 line
2.29.1
#8 lore raw

PATCH 8/14 core: kill gpiod_line_close_chip()

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

This helper was supposed to be used together with global line lookup which will soon be removed so drop this one too.

---
 include/gpiod.h | 8 --------
 lib/helpers.c   | 7 -------
 2 files changed, 15 deletions(-)
include/gpiod.h +0 -8
index c6e0b65..2e8323a 100644
@@ -1092,14 +1092,6 @@ gpiod_line_get(const char *device, unsigned int offset) GPIOD_API;
  */
 struct gpiod_line *gpiod_line_find(const char *name) GPIOD_API;
 
-/**
- * @brief Close a GPIO chip owning this line and release all resources.
- * @param line GPIO line object
- *
- * After this function returns, the line must no longer be used.
- */
-void gpiod_line_close_chip(struct gpiod_line *line) GPIOD_API;
-
 /**
  * @}
  *
lib/helpers.c +0 -7
index 3ec6c96..b3050ca 100644
@@ -424,10 +424,3 @@ out:
 
 	return NULL;
 }
-
-void gpiod_line_close_chip(struct gpiod_line *line)
-{
-	struct gpiod_chip *chip = gpiod_line_get_chip(line);
-
-	gpiod_chip_close(chip);
-}
Signature · 1 line
2.29.1
#9 lore raw

PATCH 9/14 core: kill gpiod_line_get()

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

We're dropping all interfaces for global line lookup and accessing. Lines should be always accessed via their owning chip object.

---
 include/gpiod.h    | 15 ---------------
 lib/helpers.c      | 18 ------------------
 tests/tests-line.c | 21 ---------------------
 3 files changed, 54 deletions(-)
include/gpiod.h +0 -15
index 2e8323a..c1113bf 100644
@@ -1062,21 +1062,6 @@ int gpiod_line_event_read_fd_multiple(int fd, struct gpiod_line_event *events,
  * Functions that didn't fit anywhere else.
  */
 
-/**
- * @brief Get a GPIO line handle by GPIO chip description and offset.
- * @param device String describing the gpiochip.
- * @param offset The offset of the GPIO line.
- * @return GPIO line handle or NULL if an error occurred.
- *
- * This routine provides a shorter alternative to calling
- * ::gpiod_chip_open_lookup and ::gpiod_chip_get_line.
- *
- * If this function succeeds, the caller is responsible for closing the
- * associated GPIO chip.
- */
-struct gpiod_line *
-gpiod_line_get(const char *device, unsigned int offset) GPIOD_API;
-
 /**
  * @brief Find a GPIO line by its name.
  * @param name Name of the GPIO line.
lib/helpers.c +0 -18
index b3050ca..2063c3f 100644
@@ -378,24 +378,6 @@ int gpiod_line_request_bulk_both_edges_events_flags(
 					GPIOD_LINE_REQUEST_EVENT_BOTH_EDGES);
 }
 
-struct gpiod_line *gpiod_line_get(const char *device, unsigned int offset)
-{
-	struct gpiod_chip *chip;
-	struct gpiod_line *line;
-
-	chip = gpiod_chip_open_lookup(device);
-	if (!chip)
-		return NULL;
-
-	line = gpiod_chip_get_line(chip, offset);
-	if (!line) {
-		gpiod_chip_close(chip);
-		return NULL;
-	}
-
-	return line;
-}
-
 struct gpiod_line *gpiod_line_find(const char *name)
 {
 	struct gpiod_chip_iter *iter;
tests/tests-line.c +0 -21
index 4bb5f0f..235df0f 100644
@@ -716,27 +716,6 @@ GPIOD_TEST_CASE(output_value_caching, 0, { 8 })
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 0);
 }
 
-GPIOD_TEST_CASE(get_line_helper, 0, { 16, 16, 32, 16 })
-{
-	g_autoptr(gpiod_chip_struct) chip = NULL;
-	struct gpiod_line *line;
-
-	line = gpiod_line_get(gpiod_test_chip_name(2), 18);
-	g_assert_nonnull(line);
-	gpiod_test_return_if_failed();
-	chip = gpiod_line_get_chip(line);
-	g_assert_cmpint(gpiod_line_offset(line), ==, 18);
-}
-
-GPIOD_TEST_CASE(get_line_helper_invalid_offset, 0, { 16, 16, 32, 16 })
-{
-	struct gpiod_line *line;
-
-	line = gpiod_line_get(gpiod_test_chip_name(3), 18);
-	g_assert_null(line);
-	g_assert_cmpint(errno, ==, EINVAL);
-}
-
 GPIOD_TEST_CASE(find_good, GPIOD_TEST_FLAG_NAMED_LINES, { 16, 16, 32, 16 })
 {
 	g_autoptr(gpiod_chip_struct) chip = NULL;
Signature · 1 line
2.29.1
#10 lore raw

PATCH 10/14 treewide: kill global line lookup

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Global line lookup doesn't really work correctly because GPIO line names are not unique. We'd have to return a list of matching lines. Also: not all chips may be accessible by user in which case the chip iterator will fail. We'll soon be removing chip iterators entirely so for now drop the global line lookup and let users iterate over chips themselves.

---
 bindings/cxx/examples/gpiofindcxx.cpp  | 14 ++---
 bindings/cxx/gpiod.hpp                 |  9 ----
 bindings/cxx/line.cpp                  | 15 ------
 bindings/cxx/tests/tests-line.cpp      | 19 -------
 bindings/python/gpiodmodule.c          | 75 --------------------------
 bindings/python/tests/gpiod_py_test.py | 18 -------
 include/gpiod.h                        | 24 ---------
 lib/helpers.c                          | 29 ----------
 tests/tests-line.c                     | 31 -----------
 tools/gpiofind.c                       | 31 ++++++-----
 10 files changed, 26 insertions(+), 239 deletions(-)
bindings/cxx/examples/gpiofindcxx.cpp +8 -6
index aeba29d..e9ab64a 100644
@@ -19,11 +19,13 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	auto ret = ::gpiod::find_line(argv[1]);
-	if (!ret.first)
-		return EXIT_FAILURE;
-
-	::std::cout << ret.second.name() << " " << ret.first.offset() << ::std::endl;
+	for (auto& chip: ::gpiod::make_chip_iter()) {
+		auto line = chip.find_line(argv[1]);
+		if (line) {
+			::std::cout << line.name() << " " << line.offset() << ::std::endl;
+			return EXIT_SUCCESS;
+		}
+	}
 
-	return EXIT_SUCCESS;
+	return EXIT_FAILURE;
 }
bindings/cxx/gpiod.hpp +0 -9
index 0d443b0..b258730 100644
@@ -548,15 +548,6 @@ private:
 	friend line_iter;
 };
 
-/**
- * @brief Find a GPIO line by name. Search all GPIO chips present on the system.
- * @param name Name of the line.
- * @return Returns a <line, chip> pair where line is the line with given name
- *         and chip is the line's owner. Both objects are empty if the line was
- *         not found.
- */
-GPIOD_API ::std::pair<line, chip> find_line(const ::std::string& name);
-
 /**
  * @brief Describes a single GPIO line event.
  */
bindings/cxx/line.cpp +0 -15
index 5589875..54382e2 100644
@@ -343,19 +343,4 @@ line::chip_guard::chip_guard(const line& line)
 	
 }
 
-::std::pair<line, chip> find_line(const ::std::string& name)
-{
-	::std::pair<line, chip> ret;
-
-	for (auto& it: make_chip_iter()) {
-		ret.first = it.find_line(name);
-		if (ret.first) {
-			ret.second = it;
-			break;
-		}
-	}
-
-	return ret;
-}
-
 } /* namespace gpiod */
bindings/cxx/tests/tests-line.cpp +0 -19
index 9841bea..a5499ac 100644
@@ -18,25 +18,6 @@ const ::std::string consumer = "line-test";
 
 } /* namespace */
 
-TEST_CASE("Global find_line() function works", "[line]")
-{
-	mockup::probe_guard mockup_chips({ 8, 8, 8, 8, 8 }, mockup::FLAG_NAMED_LINES);
-
-	SECTION("line found")
-	{
-		auto ret = ::gpiod::find_line("gpio-mockup-C-5");
-		REQUIRE(ret.first.offset() == 5);
-		REQUIRE(ret.first.name() == "gpio-mockup-C-5");
-		REQUIRE(ret.second.label() == "gpio-mockup-C");
-	}
-
-	SECTION("line not found")
-	{
-		auto ret = ::gpiod::find_line("nonexistent-line");
-		REQUIRE_FALSE(ret.first);
-	}
-}
-
 TEST_CASE("Line information can be correctly retrieved", "[line]")
 {
 	mockup::probe_guard mockup_chips({ 8 }, mockup::FLAG_NAMED_LINES);
bindings/python/gpiodmodule.c +0 -75

87 lines not on this page · raw message

bindings/python/tests/gpiod_py_test.py +0 -18
index e4aaadc..8534ce9 100755
@@ -234,24 +234,6 @@ class ChipGetLines(MockupTestCase):
 # Line test cases
 #
 
-class LineGlobalFindLine(MockupTestCase):
-
-    chip_sizes = ( 4, 8, 16 )
-    flags = gpiomockup.Mockup.FLAG_NAMED_LINES
-
-    def test_global_find_line_function(self):
-        line = gpiod.find_line('gpio-mockup-B-4')
-        self.assertNotEqual(line, None)
-        try:
-            self.assertEqual(line.owner().label(), 'gpio-mockup-B')
-            self.assertEqual(line.offset(), 4)
-        finally:
-            line.owner().close()
-
-    def test_global_find_line_function_nonexistent(self):
-        line = gpiod.find_line('nonexistent-line')
-        self.assertEqual(line, None)
-
 class LineInfo(MockupTestCase):
 
     chip_sizes = ( 8, )
include/gpiod.h +0 -24
index c1113bf..5aeb7cc 100644
@@ -1053,30 +1053,6 @@ int gpiod_line_event_read_fd(int fd, struct gpiod_line_event *event) GPIOD_API;
 int gpiod_line_event_read_fd_multiple(int fd, struct gpiod_line_event *events,
 				      unsigned int num_events) GPIOD_API;
 
-/**
- * @}
- *
- * @defgroup line_misc Misc line functions
- * @{
- *
- * Functions that didn't fit anywhere else.
- */
-
-/**
- * @brief Find a GPIO line by its name.
- * @param name Name of the GPIO line.
- * @return Returns the GPIO line handle if the line exists in the system or
- *         NULL if it couldn't be located or an error occurred.
- * @attention GPIO lines are not unique in the linux kernel, neither globally
- *            nor within a single chip. This function finds the first line with
- *            given name.
- *
- * If this routine succeeds, the user must manually close the GPIO chip owning
- * this line to avoid memory leaks. If the line could not be found, this
- * functions sets errno to ENOENT.
- */
-struct gpiod_line *gpiod_line_find(const char *name) GPIOD_API;
-
 /**
  * @}
  *
lib/helpers.c +0 -29
index 2063c3f..5a73736 100644
@@ -377,32 +377,3 @@ int gpiod_line_request_bulk_both_edges_events_flags(
 	return line_event_request_type_bulk(bulk, consumer, flags,
 					GPIOD_LINE_REQUEST_EVENT_BOTH_EDGES);
 }
-
-struct gpiod_line *gpiod_line_find(const char *name)
-{
-	struct gpiod_chip_iter *iter;
-	struct gpiod_chip *chip;
-	struct gpiod_line *line;
-
-	iter = gpiod_chip_iter_new();
-	if (!iter)
-		return NULL;
-
-	gpiod_foreach_chip(iter, chip) {
-		line = gpiod_chip_find_line(chip, name);
-		if (line) {
-			gpiod_chip_iter_free_noclose(iter);
-			return line;
-		}
-
-		if (errno != ENOENT)
-			goto out;
-	}
-
-	errno = ENOENT;
-
-out:
-	gpiod_chip_iter_free(iter);
-
-	return NULL;
-}
tests/tests-line.c +0 -31
index 235df0f..aee85fe 100644
@@ -716,37 +716,6 @@ GPIOD_TEST_CASE(output_value_caching, 0, { 8 })
 	g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 0);
 }
 
-GPIOD_TEST_CASE(find_good, GPIOD_TEST_FLAG_NAMED_LINES, { 16, 16, 32, 16 })
-{
-	g_autoptr(gpiod_chip_struct) chip = NULL;
-	struct gpiod_line *line;
-
-	line = gpiod_line_find("gpio-mockup-C-12");
-	g_assert_nonnull(line);
-	gpiod_test_return_if_failed();
-	chip = gpiod_line_get_chip(line);
-	g_assert_cmpint(gpiod_line_offset(line), ==, 12);
-}
-
-GPIOD_TEST_CASE(find_not_found,
-		GPIOD_TEST_FLAG_NAMED_LINES, { 16, 16, 32, 16 })
-{
-	struct gpiod_line *line;
-
-	line = gpiod_line_find("nonexistent");
-	g_assert_null(line);
-	g_assert_cmpint(errno, ==, ENOENT);
-}
-
-GPIOD_TEST_CASE(find_unnamed_lines, 0, { 16, 16, 32, 16 })
-{
-	struct gpiod_line *line;
-
-	line = gpiod_line_find("gpio-mockup-C-12");
-	g_assert_null(line);
-	g_assert_cmpint(errno, ==, ENOENT);
-}
-
 GPIOD_TEST_CASE(direction, 0, { 8 })
 {
 	g_autoptr(gpiod_chip_struct) chip = NULL;
tools/gpiofind.c +18 -13
index 2138ebf..489cf33 100644
@@ -34,9 +34,10 @@ static void print_help(void)
 
 int main(int argc, char **argv)
 {
-	int optc, opti, ret = EXIT_SUCCESS;
+	struct gpiod_chip_iter *iter;
 	struct gpiod_chip *chip;
 	struct gpiod_line *line;
+	int optc, opti;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -63,19 +64,23 @@ int main(int argc, char **argv)
 	if (argc != 1)
 		die("exactly one GPIO line name must be specified");
 
-	line = gpiod_line_find(argv[0]);
-	if (!line) {
-		if (errno == ENOENT)
-			return EXIT_FAILURE;
+	iter = gpiod_chip_iter_new();
+	if (!iter)
+		die_perror("unable to access GPIO chips");
 
-		die_perror("error performing the line lookup");
-	}
-
-	chip = gpiod_line_get_chip(line);
-
-	printf("%s %u\n", gpiod_chip_name(chip), gpiod_line_offset(line));
+	gpiod_foreach_chip(iter, chip) {
+		line = gpiod_chip_find_line(chip, argv[0]);
+		if (line) {
+			printf("%s %u\n",
+			       gpiod_chip_name(chip), gpiod_line_offset(line));
+			gpiod_chip_iter_free(iter);
+			return EXIT_SUCCESS;
+		}
 
-	gpiod_chip_close(chip);
+		if (errno != ENOENT)
+			die_perror("error performing the line lookup");
+	}
 
-	return ret;
+	gpiod_chip_iter_free(iter);
+	return EXIT_FAILURE;
 }
Signature · 1 line
2.29.1
#11 lore raw

PATCH 11/14 treewide: kill find_lines()

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

GPIO line names are not unique. Looking up multiple lines by names would require us to return a list of matching lines for every name. We're simplifying the library API so drop this interface treewide.

---
 bindings/cxx/chip.cpp                  | 18 ------
 bindings/cxx/gpiod.hpp                 |  7 ---
 bindings/cxx/tests/tests-chip.cpp      | 18 ------
 bindings/python/gpiodmodule.c          | 86 --------------------------
 bindings/python/tests/gpiod_py_test.py | 32 ----------
 include/gpiod.h                        | 15 -----
 lib/helpers.c                          | 28 ---------
 tests/tests-chip.c                     | 49 ---------------
 8 files changed, 253 deletions(-)
bindings/cxx/chip.cpp +0 -18
index dffa7a2..ff35e53 100644
@@ -154,24 +154,6 @@ line_bulk chip::get_all_lines(void) const
 	return lines;
 }
 
-line_bulk chip::find_lines(const ::std::vector<::std::string>& names) const
-{
-	line_bulk lines;
-	line line;
-
-	for (auto& it: names) {
-		line = this->find_line(it);
-		if (!line) {
-			lines.clear();
-			return lines;
-		}
-
-		lines.append(line);
-	}
-
-	return lines;
-}
-
 bool chip::operator==(const chip& rhs) const noexcept
 {
 	return this->_m_chip.get() == rhs._m_chip.get();
bindings/cxx/gpiod.hpp +0 -7
index b258730..a16a27c 100644
@@ -146,13 +146,6 @@ public:
 	 */
 	GPIOD_API line_bulk get_all_lines(void) const;
 
-	/**
-	 * @brief Get a set of lines exposed by this chip by their names.
-	 * @param names Vector of line names.
-	 * @return Set of lines held by a line_bulk object.
-	 */
-	GPIOD_API line_bulk find_lines(const ::std::vector<::std::string>& names) const;
-
 	/**
 	 * @brief Equality operator.
 	 * @param rhs Right-hand side of the equation.
bindings/cxx/tests/tests-chip.cpp +0 -18
index 4c9f113..90ebc1b 100644
@@ -191,17 +191,6 @@ TEST_CASE("Lines can be retrieved from chip objects", "[chip]")
 		REQUIRE(lines.get(2).name() == "gpio-mockup-B-3");
 		REQUIRE(lines.get(3).name() == "gpio-mockup-B-2");
 	}
-
-	SECTION("find multiple lines by names")
-	{
-		auto lines = chip.find_lines({ "gpio-mockup-B-2",
-					       "gpio-mockup-B-5",
-					       "gpio-mockup-B-6"});
-		REQUIRE(lines.size() == 3);
-		REQUIRE(lines.get(0).offset() == 2);
-		REQUIRE(lines.get(1).offset() == 5);
-		REQUIRE(lines.get(2).offset() == 6);
-	}
 }
 
 TEST_CASE("All lines can be retrieved from a chip at once", "[chip]")
@@ -236,11 +225,4 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
 	{
 		REQUIRE_FALSE(chip.find_line("nonexistent-line"));
 	}
-
-	SECTION("line not found by name (multiple lines)")
-	{
-		REQUIRE_FALSE(chip.find_lines({ "gpio-mockup-B-2",
-						"nonexistent-line",
-						"gpio-mockup-B-6"}));
-	}
 }
bindings/python/gpiodmodule.c +0 -86

98 lines not on this page · raw message

bindings/python/tests/gpiod_py_test.py +0 -32
index 8534ce9..6ee72a9 100755
@@ -182,30 +182,6 @@ class ChipGetLines(MockupTestCase):
             self.assertEqual(lines[2].name(), 'gpio-mockup-B-6')
             self.assertEqual(lines[3].name(), 'gpio-mockup-B-7')
 
-    def test_find_multiple_lines_by_names_in_tuple(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
-            lines = chip.find_lines(( 'gpio-mockup-B-0',
-                                      'gpio-mockup-B-3',
-                                      'gpio-mockup-B-4',
-                                      'gpio-mockup-B-6' )).to_list()
-            self.assertEqual(len(lines), 4)
-            self.assertEqual(lines[0].offset(), 0)
-            self.assertEqual(lines[1].offset(), 3)
-            self.assertEqual(lines[2].offset(), 4)
-            self.assertEqual(lines[3].offset(), 6)
-
-    def test_find_multiple_lines_by_names_in_list(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
-            lines = chip.find_lines([ 'gpio-mockup-B-0',
-                                      'gpio-mockup-B-3',
-                                      'gpio-mockup-B-4',
-                                      'gpio-mockup-B-6' ]).to_list()
-            self.assertEqual(len(lines), 4)
-            self.assertEqual(lines[0].offset(), 0)
-            self.assertEqual(lines[1].offset(), 3)
-            self.assertEqual(lines[2].offset(), 4)
-            self.assertEqual(lines[3].offset(), 6)
-
     def test_get_multiple_lines_invalid_offset(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
             with self.assertRaises(OSError) as err_ctx:
@@ -213,14 +189,6 @@ class ChipGetLines(MockupTestCase):
 
             self.assertEqual(err_ctx.exception.errno, errno.EINVAL)
 
-    def test_find_multiple_lines_nonexistent(self):
-        with gpiod.Chip(mockup.chip_name(1)) as chip:
-            with self.assertRaises(TypeError):
-                lines = chip.find_lines(( 'gpio-mockup-B-0',
-                                          'nonexistent-line',
-                                          'gpio-mockup-B-4',
-                                          'gpio-mockup-B-6' )).to_list()
-
     def test_get_all_lines(self):
         with gpiod.Chip(mockup.chip_name(2)) as chip:
             lines = chip.get_all_lines().to_list()
include/gpiod.h +0 -15
index 5aeb7cc..34313ed 100644
@@ -184,21 +184,6 @@ gpiod_chip_get_all_lines(struct gpiod_chip *chip) GPIOD_API;
 struct gpiod_line *
 gpiod_chip_find_line(struct gpiod_chip *chip, const char *name) GPIOD_API;
 
-/**
- * @brief Find a set of GPIO lines by names among lines exposed by this chip.
- * @param chip The GPIO chip object.
- * @param names Array of pointers to C-strings containing the names of the
- *              lines to lookup. Must end with a NULL-pointer.
- * @return New line bulk object or NULL on error.
- * @note If at least one line from the list could not be found among the lines
- *       exposed by this chip, the function sets errno to ENOENT.
- * @attention GPIO line names are not unique in the linux kernel, neither
- *            globally nor within a single chip. This function finds the first
- *            line with given name.
- */
-struct gpiod_line_bulk *
-gpiod_chip_find_lines(struct gpiod_chip *chip, const char **names) GPIOD_API;
-
 /**
  * @}
  *
lib/helpers.c +0 -28
index 5a73736..509a1c8 100644
@@ -142,34 +142,6 @@ gpiod_chip_find_line(struct gpiod_chip *chip, const char *name)
 	return NULL;
 }
 
-struct gpiod_line_bulk *
-gpiod_chip_find_lines(struct gpiod_chip *chip, const char **names)
-{
-	struct gpiod_line_bulk *bulk;
-	struct gpiod_line *line;
-	unsigned int num_names;
-	int i;
-
-	for (i = 0; names[i]; i++);
-	num_names = i;
-
-	bulk = gpiod_line_bulk_new(num_names);
-	if (!bulk)
-		return NULL;
-
-	for (i = 0; names[i]; i++) {
-		line = gpiod_chip_find_line(chip, names[i]);
-		if (!line) {
-			gpiod_line_bulk_free(bulk);
-			return NULL;
-		}
-
-		gpiod_line_bulk_add_line(bulk, line);
-	}
-
-	return bulk;
-}
-
 int gpiod_line_request_input(struct gpiod_line *line, const char *consumer)
 {
 	struct gpiod_line_request_config config = {
tests/tests-chip.c +0 -49

52 lines not on this page · raw message

Signature · 1 line
2.29.1
#12 lore raw

PATCH 12/14 core: rework gpiod_chip_find_line()

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

GPIO line names are not unique and the library has been incorrectly making such assumption in v1.x series. We've already dropped interfaces that would be too complicated to maintain when taking this into account.

Let's now rework the remaming API. This introduces a new function to the C API: gpiod_chip_find_line_unique() which assumes that the line name is unique and signals an error if it's not. The previous gpiod_chip_find_line() function now returns a bulk object containing all matching lines.

Python and C++ bindings are updated: their find_line() functions return bulk objects (or an ::std::vector> too now and they take an additional argument specifying whether we're looking for a unique line or not.

---
 bindings/cxx/chip.cpp                  |  18 ++-
 bindings/cxx/examples/gpiofindcxx.cpp  |   6 +-
 bindings/cxx/gpiod.hpp                 |  10 +-
 bindings/cxx/tests/tests-chip.cpp      |   4 +-
 bindings/python/gpiodmodule.c          | 150 ++++++++++++++-----------
 bindings/python/tests/gpiod_py_test.py |   9 +-
 include/gpiod.h                        |  28 +++--
 lib/helpers.c                          |  57 +++++++++-
 tests/tests-chip.c                     |   9 +-
 tools/gpiofind.c                       |   2 +-
 10 files changed, 193 insertions(+), 100 deletions(-)
bindings/cxx/chip.cpp +12 -6
index ff35e53..82ba559 100644
@@ -122,16 +122,22 @@ line chip::get_line(unsigned int offset) const
 	return line(line_handle, *this);
 }
 
-line chip::find_line(const ::std::string& name) const
+::std::vector<line> chip::find_line(const ::std::string& name, bool unique) const
 {
 	this->throw_if_noref();
 
-	::gpiod_line* handle = ::gpiod_chip_find_line(this->_m_chip.get(), name.c_str());
-	if (!handle && errno != ENOENT)
-		throw ::std::system_error(errno, ::std::system_category(),
-					  "error looking up GPIO line by name");
+	::std::vector<line> lines;
+
+	for (auto& line: ::gpiod::line_iter(*this)) {
+		if (line.name() == name)
+			lines.push_back(line);
+	}
 
-	return handle ? line(handle, *this) : line();
+	if (unique && lines.size() > 1)
+		throw ::std::system_error(ERANGE, ::std::system_category(),
+					  "multiple lines with the same name found");
+
+	return lines;
 }
 
 line_bulk chip::get_lines(const ::std::vector<unsigned int>& offsets) const
bindings/cxx/examples/gpiofindcxx.cpp +3 -3
index e9ab64a..c817378 100644
@@ -20,9 +20,9 @@ int main(int argc, char **argv)
 	}
 
 	for (auto& chip: ::gpiod::make_chip_iter()) {
-		auto line = chip.find_line(argv[1]);
-		if (line) {
-			::std::cout << line.name() << " " << line.offset() << ::std::endl;
+		auto lines = chip.find_line(argv[1], true);
+		if (!lines.empty()) {
+			::std::cout << lines.front().name() << " " << lines.front().offset() << ::std::endl;
 			return EXIT_SUCCESS;
 		}
 	}
bindings/cxx/gpiod.hpp +7 -3
index a16a27c..9d081fe 100644
@@ -127,11 +127,15 @@ public:
 	GPIOD_API line get_line(unsigned int offset) const;
 
 	/**
-	 * @brief Get the line exposed by this chip by name.
+	 * @brief Find all GPIO lines by name among lines exposed by this GPIO
+	 *        chip.
 	 * @param name Line name.
-	 * @return Line object.
+	 * @param unique If set to true: throw an error if multiple lines match
+	 *               the name.
+	 * @return Vector of all matching lines.
 	 */
-	GPIOD_API line find_line(const ::std::string& name) const;
+	GPIOD_API ::std::vector<line> find_line(const ::std::string& name,
+						bool unique = false) const;
 
 	/**
 	 * @brief Get a set of lines exposed by this chip at given offsets.
bindings/cxx/tests/tests-chip.cpp +2 -2
index 90ebc1b..c45f2df 100644
@@ -168,7 +168,7 @@ TEST_CASE("Lines can be retrieved from chip objects", "[chip]")
 
 	SECTION("find single line by name")
 	{
-		auto line = chip.find_line("gpio-mockup-B-3");
+		auto line = chip.find_line("gpio-mockup-B-3", true).front();
 		REQUIRE(line.offset() == 3);
 	}
 
@@ -223,6 +223,6 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
 
 	SECTION("line not found by name")
 	{
-		REQUIRE_FALSE(chip.find_line("nonexistent-line"));
+		REQUIRE(chip.find_line("nonexistent-line").empty());
 	}
 }
bindings/python/gpiodmodule.c +86 -64

201 lines not on this page · raw message

bindings/python/tests/gpiod_py_test.py +5 -4
index 6ee72a9..c490933 100755
@@ -149,8 +149,9 @@ class ChipGetLines(MockupTestCase):
 
     def test_find_single_line_by_name(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
-            line = chip.find_line('gpio-mockup-B-4')
-            self.assertEqual(line.offset(), 4)
+            lines = chip.find_line('gpio-mockup-B-4').to_list()
+            self.assertEqual(len(lines), 1)
+            self.assertEqual(lines[0].offset(), 4)
 
     def test_get_single_line_invalid_offset(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
@@ -161,8 +162,8 @@ class ChipGetLines(MockupTestCase):
 
     def test_find_single_line_nonexistent(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
-            line = chip.find_line('nonexistent-line')
-            self.assertEqual(line, None)
+            lines = chip.find_line('nonexistent-line')
+            self.assertEqual(lines, None)
 
     def test_get_multiple_lines_by_offsets_in_tuple(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
include/gpiod.h +20 -8
index 34313ed..fc50fe8 100644
@@ -170,19 +170,31 @@ struct gpiod_line_bulk *
 gpiod_chip_get_all_lines(struct gpiod_chip *chip) GPIOD_API;
 
 /**
- * @brief Find a GPIO line by name among lines associated with given GPIO chip.
+ * @brief Find all GPIO lines by name among lines exposed by this GPIO chip.
  * @param chip The GPIO chip object.
- * @param name The name of the GPIO line.
+ * @param name GPIO line name to look for.
+ * @return New line bulk object containing all matching lines or NULL on error.
+ *
+ * If no line with given name is associated with this chip, the function sets
+ * errno to ENOENT.
+ */
+struct gpiod_line_bulk *
+gpiod_chip_find_line(struct gpiod_chip *chip, const char *name) GPIOD_API;
+
+/**
+ * @brief Find a unique line by name among lines exposed by this GPIO chip.
+ * @param chip The GPIO chip object.
+ * @param name Name of the GPIO line.
  * @return Pointer to the GPIO line handle or NULL if the line could not be
  *         found or an error occurred.
- * @note In case a line with given name is not associated with given chip, the
- *       function sets errno to ENOENT.
- * @attention GPIO line names are not unique in the linux kernel, neither
- *            globally nor within a single chip. This function finds the first
- *            line with given name.
+ *
+ * If no line with given name is associated with this chip, the function sets
+ * errno to ENOENT. If more than one line with given name is associated with
+ * this chip, the function sets errno to ERANGE.
  */
 struct gpiod_line *
-gpiod_chip_find_line(struct gpiod_chip *chip, const char *name) GPIOD_API;
+gpiod_chip_find_line_unique(struct gpiod_chip *chip,
+			    const char *name) GPIOD_API;
 
 /**
  * @}
lib/helpers.c +52 -5

75 lines not on this page · raw message

tests/tests-chip.c +5 -4
index 543c103..1c365b9 100644
@@ -235,7 +235,7 @@ GPIOD_TEST_CASE(get_all_lines, 0, { 4 })
 	g_assert_cmpuint(gpiod_line_offset(line3), ==, 3);
 }
 
-GPIOD_TEST_CASE(find_line_good, GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
+GPIOD_TEST_CASE(find_line_good_unique, GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
 {
 	g_autoptr(gpiod_chip_struct) chip = NULL;
 	struct gpiod_line *line;
@@ -244,14 +244,15 @@ GPIOD_TEST_CASE(find_line_good, GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
 	g_assert_nonnull(chip);
 	gpiod_test_return_if_failed();
 
-	line = gpiod_chip_find_line(chip, "gpio-mockup-B-4");
+	line = gpiod_chip_find_line_unique(chip, "gpio-mockup-B-4");
 	g_assert_nonnull(line);
 	gpiod_test_return_if_failed();
 	g_assert_cmpuint(gpiod_line_offset(line), ==, 4);
 	g_assert_cmpstr(gpiod_line_name(line), ==, "gpio-mockup-B-4");
 }
 
-GPIOD_TEST_CASE(find_line_not_found, GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
+GPIOD_TEST_CASE(find_line_unique_not_found,
+		GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
 {
 	g_autoptr(gpiod_chip_struct) chip = NULL;
 	struct gpiod_line *line;
@@ -260,7 +261,7 @@ GPIOD_TEST_CASE(find_line_not_found, GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
 	g_assert_nonnull(chip);
 	gpiod_test_return_if_failed();
 
-	line = gpiod_chip_find_line(chip, "nonexistent");
+	line = gpiod_chip_find_line_unique(chip, "nonexistent");
 	g_assert_null(line);
 	g_assert_cmpint(errno, ==, ENOENT);
 }
tools/gpiofind.c +1 -1
index 489cf33..ffb8fc0 100644
@@ -69,7 +69,7 @@ int main(int argc, char **argv)
 		die_perror("unable to access GPIO chips");
 
 	gpiod_foreach_chip(iter, chip) {
-		line = gpiod_chip_find_line(chip, argv[0]);
+		line = gpiod_chip_find_line_unique(chip, argv[0]);
 		if (line) {
 			printf("%s %u\n",
 			       gpiod_chip_name(chip), gpiod_line_offset(line));
Signature · 1 line
2.29.1
#13 lore raw

PATCH 13/14 build: add a configure switch for building examples

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Example code for bindings is currently always built if bindings are enabled. Make it conditional with a new configure switch.

---
 bindings/cxx/Makefile.am    |  8 +++++++-
 bindings/python/Makefile.am | 10 ++++++++--
 configure.ac                |  6 ++++++
 3 files changed, 21 insertions(+), 3 deletions(-)
bindings/cxx/Makefile.am +7 -1
index 5c40ceb..87463b0 100644
@@ -18,10 +18,16 @@ include_HEADERS = gpiod.hpp
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = libgpiodcxx.pc
 
-SUBDIRS = . examples
+SUBDIRS = .
 
 if WITH_TESTS
 
 SUBDIRS += tests
 
 endif
+
+if WITH_EXAMPLES
+
+SUBDIRS += examples
+
+endif
bindings/python/Makefile.am +8 -2
index 124f152..5403bcb 100644
@@ -6,8 +6,6 @@
 # Copyright (C) 2017-2018 Bartosz Golaszewski <[email protected]>
 #
 
-SUBDIRS = . examples
-
 pyexec_LTLIBRARIES = gpiod.la
 
 gpiod_la_SOURCES = gpiodmodule.c
@@ -17,8 +15,16 @@ gpiod_la_CFLAGS += -Wall -Wextra -g -std=gnu89 $(PYTHON_CPPFLAGS)
 gpiod_la_LDFLAGS = -module -avoid-version
 gpiod_la_LIBADD = $(top_builddir)/lib/libgpiod.la $(PYTHON_LIBS)
 
+SUBDIRS = .
+
 if WITH_TESTS
 
 SUBDIRS += tests
 
 endif
+
+if WITH_EXAMPLES
+
+SUBDIRS += examples
+
+endif
configure.ac +6 -0
index 57c99a8..90a6324 100644
@@ -155,6 +155,12 @@ then
 	fi
 fi
 
+AC_ARG_ENABLE([examples],
+	[AS_HELP_STRING([--enable-examples], [enable building code examples[default=no]])],
+	[if test "x$enableval" = xyes; then with_examples=true; fi],
+	[with_examples=false])
+AM_CONDITIONAL([WITH_EXAMPLES], [test "x$with_examples" = xtrue])
+
 AC_ARG_ENABLE([bindings-cxx],
 	[AS_HELP_STRING([--enable-bindings-cxx],[enable C++ bindings [default=no]])],
 	[if test "x$enableval" = xyes; then with_bindings_cxx=true; fi],
Signature · 1 line
2.29.1
#14 lore raw

PATCH 14/14 core: kill chip iterators

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Chip iterators require the user to have permission to access all GPIO chips. They also don't take into account symbolic links. In general they're badly designed so remove them treewide in favor of scanning /dev manually using the provided gpiod_is_gpiochip_device() helper.

---
 bindings/cxx/chip.cpp                   |   5 +
 bindings/cxx/examples/Makefile.am       |   4 +-
 bindings/cxx/examples/gpiodetectcxx.cpp |  13 ++-
 bindings/cxx/examples/gpiofindcxx.cpp   |  16 ++-
 bindings/cxx/examples/gpioinfocxx.cpp   |  43 +++++----
 bindings/cxx/gpiod.hpp                  | 117 ++--------------------
 bindings/cxx/iter.cpp                   |  67 -------------
 bindings/cxx/tests/tests-chip.cpp       |  20 ++++
 bindings/cxx/tests/tests-iter.cpp       |  37 -------
 bindings/python/examples/gpiodetect.py  |  12 ++-
 bindings/python/examples/gpiofind.py    |  15 ++-
 bindings/python/examples/gpioinfo.py    |  33 ++++---
 bindings/python/gpiodmodule.c           | 114 +++++++---------------
 bindings/python/tests/gpiod_py_test.py  |  39 +++-----
 configure.ac                            |   6 ++
 include/gpiod.h                         |  93 +-----------------
 lib/Makefile.am                         |   2 +-
 lib/iter.c                              | 123 ------------------------
 tests/Makefile.am                       |   1 -
 tests/gpiod-test.h                      |   2 -
 tests/tests-iter.c                      | 100 -------------------
 tools/gpiodetect.c                      |  29 ++++--
 tools/gpiofind.c                        |  27 +++---
 tools/gpioinfo.c                        |  27 ++++--
 tools/tools-common.c                    |  15 +++
 tools/tools-common.h                    |   3 +
 26 files changed, 250 insertions(+), 713 deletions(-)
 delete mode 100644 lib/iter.c
 delete mode 100644 tests/tests-iter.c
bindings/cxx/chip.cpp +5 -0
index 82ba559..107088e 100644
@@ -51,6 +51,11 @@ void chip_deleter(::gpiod_chip* chip)
 
 } /* namespace */
 
+bool is_gpiochip_device(const ::std::string& path)
+{
+	return ::gpiod_is_gpiochip_device(path.c_str());
+}
+
 chip::chip(const ::std::string& device, int how)
 	: _m_chip()
 {
bindings/cxx/examples/Makefile.am +2 -2
index 8d39be2..43e9875 100644
@@ -7,8 +7,8 @@
 #
 
 AM_CPPFLAGS = -I$(top_srcdir)/bindings/cxx/ -I$(top_srcdir)/include
-AM_CPPFLAGS += -Wall -Wextra -g -std=gnu++11
-AM_LDFLAGS = -lgpiodcxx -L$(top_builddir)/bindings/cxx/
+AM_CPPFLAGS += -Wall -Wextra -g -std=gnu++17
+AM_LDFLAGS = -lgpiodcxx -L$(top_builddir)/bindings/cxx/ -lstdc++fs
 
 noinst_PROGRAMS =				\
 		gpiodetectcxx			\
bindings/cxx/examples/gpiodetectcxx.cpp +9 -4
index 6da5573..76d367d 100644
@@ -10,6 +10,7 @@
 #include <gpiod.hpp>
 
 #include <cstdlib>
+#include <filesystem>
 #include <iostream>
 
 int main(int argc, char **argv)
@@ -19,10 +20,14 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	for (auto& it: ::gpiod::make_chip_iter()) {
-	        ::std::cout << it.name() << " ["
-			  << it.label() << "] ("
-			  << it.num_lines() << " lines)" << ::std::endl;
+	for (const auto& entry: ::std::filesystem::directory_iterator("/dev/")) {
+		if (::gpiod::is_gpiochip_device(entry.path())) {
+			::gpiod::chip chip(entry.path());
+
+			::std::cout << chip.name() << " ["
+				    << chip.label() << "] ("
+				    << chip.num_lines() << " lines)" << ::std::endl;
+		}
 	}
 
 	return EXIT_SUCCESS;
bindings/cxx/examples/gpiofindcxx.cpp +11 -5
index c817378..f8b771c 100644
@@ -10,6 +10,7 @@
 #include <gpiod.hpp>
 
 #include <cstdlib>
+#include <filesystem>
 #include <iostream>
 
 int main(int argc, char **argv)
@@ -19,11 +20,16 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	for (auto& chip: ::gpiod::make_chip_iter()) {
-		auto lines = chip.find_line(argv[1], true);
-		if (!lines.empty()) {
-			::std::cout << lines.front().name() << " " << lines.front().offset() << ::std::endl;
-			return EXIT_SUCCESS;
+	for (const auto& entry: ::std::filesystem::directory_iterator("/dev/")) {
+		if (::gpiod::is_gpiochip_device(entry.path())) {
+			::gpiod::chip chip(entry.path());
+
+			auto lines = chip.find_line(argv[1], true);
+			if (!lines.empty()) {
+				::std::cout << lines.front().name() << " " <<
+					       lines.front().offset() << ::std::endl;
+				return EXIT_SUCCESS;
+			}
 		}
 	}
 
bindings/cxx/examples/gpioinfocxx.cpp +24 -19

61 lines not on this page · raw message

bindings/cxx/gpiod.hpp +8 -109

129 lines not on this page · raw message

bindings/cxx/iter.cpp +0 -67

73 lines not on this page · raw message

bindings/cxx/tests/tests-chip.cpp +20 -0
index c45f2df..2492b42 100644
@@ -12,6 +12,26 @@
 
 using ::gpiod::test::mockup;
 
+TEST_CASE("GPIO chip device can be verified with is_gpiochip_device()", "[chip]")
+{
+	mockup::probe_guard mockup_chips({ 8 });
+
+	SECTION("good chip")
+	{
+		REQUIRE(::gpiod::is_gpiochip_device(mockup::instance().chip_path(0)));
+	}
+
+	SECTION("not a chip")
+	{
+		REQUIRE_FALSE(::gpiod::is_gpiochip_device("/dev/null"));
+	}
+
+	SECTION("nonexistent file")
+	{
+		REQUIRE_FALSE(::gpiod::is_gpiochip_device("/dev/nonexistent_device"));
+	}
+}
+
 TEST_CASE("GPIO chip device can be opened in different modes", "[chip]")
 {
 	mockup::probe_guard mockup_chips({ 8, 8, 8 });
bindings/cxx/tests/tests-iter.cpp +0 -37
index 4c07613..708709c 100644
@@ -12,43 +12,6 @@
 
 using ::gpiod::test::mockup;
 
-TEST_CASE("Chip iterator works", "[iter][chip]")
-{
-	mockup::probe_guard mockup_chips({ 8, 8, 8 });
-	bool gotA = false, gotB = false, gotC = false;
-
-	for (auto& it: ::gpiod::make_chip_iter()) {
-		if (it.label() == "gpio-mockup-A")
-			gotA = true;
-		if (it.label() == "gpio-mockup-B")
-			gotB = true;
-		if (it.label() == "gpio-mockup-C")
-			gotC = true;
-	}
-
-	REQUIRE(gotA);
-	REQUIRE(gotB);
-	REQUIRE(gotC);
-}
-
-TEST_CASE("Chip iterator loop can be broken out of", "[iter][chip]")
-{
-	mockup::probe_guard mockup_chips({ 8, 8, 8, 8, 8, 8 });
-	int count_chips = 0;
-
-	for (auto& it: ::gpiod::make_chip_iter()) {
-		if (it.label() == "gpio-mockup-A" ||
-		    it.label() == "gpio-mockup-B" ||
-		    it.label() == "gpio-mockup-C")
-			count_chips++;
-
-		if (count_chips == 3)
-			break;
-	}
-
-	REQUIRE(count_chips == 3);
-}
-
 TEST_CASE("Line iterator works", "[iter][line]")
 {
 	mockup::probe_guard mockup_chips({ 4 });
bindings/python/examples/gpiodetect.py +7 -5
index 9318f51..781939b 100755
@@ -10,10 +10,12 @@
 '''Reimplementation of the gpiodetect tool in Python.'''
 
 import gpiod
+import os
 
 if __name__ == '__main__':
-    for chip in gpiod.ChipIter():
-        print('{} [{}] ({} lines)'.format(chip.name(),
-                                          chip.label(),
-                                          chip.num_lines()))
-        chip.close()
+    for entry in os.scandir('/dev/'):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                print('{} [{}] ({} lines)'.format(chip.name(),
+                                                  chip.label(),
+                                                  chip.num_lines()))
bindings/python/examples/gpiofind.py +10 -5
index 8505ba0..2655742 100755
@@ -10,12 +10,17 @@
 '''Reimplementation of the gpiofind tool in Python.'''
 
 import gpiod
+import os
 import sys
 
 if __name__ == '__main__':
-    line = gpiod.find_line(sys.argv[1])
-    if line is None:
-        sys.exit(1)
+    for entry in os.scandir('/dev/'):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                lines = chip.find_line(sys.argv[1], unique=True)
+                if lines is not None:
+                     line = lines.to_list()[0]
+                     print('{} {}'.format(line.owner().name(), line.offset()))
+                     sys.exit(0)
 
-    print('{} {}'.format(line.owner().name(), line.offset()))
-    line.owner().close()
+    sys.exit(1)
bindings/python/examples/gpioinfo.py +17 -16
index de4b6f3..6a47b66 100755
@@ -10,23 +10,24 @@
 '''Simplified reimplementation of the gpioinfo tool in Python.'''
 
 import gpiod
+import os
 
 if __name__ == '__main__':
-    for chip in gpiod.ChipIter():
-        print('{} - {} lines:'.format(chip.name(), chip.num_lines()))
+    for entry in os.scandir('/dev/'):
+        if gpiod.is_gpiochip_device(entry.path):
+            with gpiod.Chip(entry.path) as chip:
+                print('{} - {} lines:'.format(chip.name(), chip.num_lines()))
 
-        for line in gpiod.LineIter(chip):
-            offset = line.offset()
-            name = line.name()
-            consumer = line.consumer()
-            direction = line.direction()
-            active_state = line.active_state()
+                for line in gpiod.LineIter(chip):
+                    offset = line.offset()
+                    name = line.name()
+                    consumer = line.consumer()
+                    direction = line.direction()
+                    active_state = line.active_state()
 
-            print('\tline {:>3}: {:>18} {:>12} {:>8} {:>10}'.format(
-                    offset,
-                    'unnamed' if name is None else name,
-                    'unused' if consumer is None else consumer,
-                    'input' if direction == gpiod.Line.DIRECTION_INPUT else 'output',
-                    'active-low' if active_state == gpiod.Line.ACTIVE_LOW else 'active-high'))
-
-        chip.close()
+                    print('\tline {:>3}: {:>18} {:>12} {:>8} {:>10}'.format(
+                          offset,
+                          'unnamed' if name is None else name,
+                          'unused' if consumer is None else consumer,
+                          'input' if direction == gpiod.Line.DIRECTION_INPUT else 'output',
+                          'active-low' if active_state == gpiod.Line.ACTIVE_LOW else 'active-high'))
bindings/python/gpiodmodule.c +37 -77

150 lines not on this page · raw message

bindings/python/tests/gpiod_py_test.py +15 -24
index c490933..f116657 100755
@@ -79,6 +79,19 @@ def check_kernel(major, minor, release):
 # Chip test cases
 #
 
+class IsGpioDevice(MockupTestCase):
+
+    chip_sizes = ( 8, )
+
+    def test_is_gpiochip_device_good(self):
+        self.assertTrue(gpiod.is_gpiochip_device(mockup.chip_path(0)))
+
+    def test_is_gpiochip_device_bad(self):
+        self.assertFalse(gpiod.is_gpiochip_device('/dev/null'))
+
+    def test_is_gpiochip_device_nonexistent(self):
+        self.assertFalse(gpiod.is_gpiochip_device('/dev/nonexistent_device'))
+
 class ChipOpen(MockupTestCase):
 
     chip_sizes = ( 8, 8, 8 )
@@ -149,8 +162,7 @@ class ChipGetLines(MockupTestCase):
 
     def test_find_single_line_by_name(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
-            lines = chip.find_line('gpio-mockup-B-4').to_list()
-            self.assertEqual(len(lines), 1)
+            lines = chip.find_line('gpio-mockup-B-4', unique=True).to_list()
             self.assertEqual(lines[0].offset(), 4)
 
     def test_get_single_line_invalid_offset(self):
@@ -162,7 +174,7 @@ class ChipGetLines(MockupTestCase):
 
     def test_find_single_line_nonexistent(self):
         with gpiod.Chip(mockup.chip_name(1)) as chip:
-            lines = chip.find_line('nonexistent-line')
+            lines = chip.find_line('nonexistent-line', unique=True)
             self.assertEqual(lines, None)
 
     def test_get_multiple_lines_by_offsets_in_tuple(self):
@@ -675,27 +687,6 @@ class LineRequestBehavior(MockupTestCase):
 # Iterator test cases
 #
 
-class ChipIterator(MockupTestCase):
-
-    chip_sizes = ( 4, 8, 16 )
-
-    def test_iterate_over_chips(self):
-        gotA = False
-        gotB = False
-        gotC = False
-
-        for chip in gpiod.ChipIter():
-            if chip.label() == 'gpio-mockup-A':
-                gotA = True
-            elif chip.label() == 'gpio-mockup-B':
-                gotB = True
-            elif chip.label() == 'gpio-mockup-C':
-                gotC = True
-
-        self.assertTrue(gotA)
-        self.assertTrue(gotB)
-        self.assertTrue(gotC)
-
 class LineIterator(MockupTestCase):
 
     chip_sizes = ( 4, )
configure.ac +6 -0
index 90a6324..ddb9dc2 100644
@@ -181,6 +181,12 @@ then
 			AC_LANG_POP([C++])
 		])
 	fi
+
+	if test "x$with_examples" = xtrue
+	then
+		# Examples use C++17 features
+		AX_CXX_COMPILE_STDCXX([17], [ext], [mandatory])
+	fi
 fi
 
 AC_ARG_ENABLE([bindings-python],
include/gpiod.h +2 -91

117 lines not on this page · raw message

lib/Makefile.am +1 -1
index c5277ce..43ebf76 100644
@@ -7,7 +7,7 @@
 #
 
 lib_LTLIBRARIES = libgpiod.la
-libgpiod_la_SOURCES = core.c helpers.c iter.c misc.c
+libgpiod_la_SOURCES = core.c helpers.c misc.c
 libgpiod_la_CFLAGS = -Wall -Wextra -g -std=gnu89
 libgpiod_la_CFLAGS += -fvisibility=hidden -I$(top_srcdir)/include/
 libgpiod_la_CFLAGS += -include $(top_builddir)/config.h
lib/iter.c deleted +0 -123

123 lines not on this page · raw message

tests/Makefile.am +0 -1
index 2d3b959..91798f7 100644
@@ -25,6 +25,5 @@ gpiod_test_SOURCES =			\
 		tests-bulk.c		\
 		tests-chip.c		\
 		tests-event.c		\
-		tests-iter.c		\
 		tests-line.c		\
 		tests-misc.c
tests/gpiod-test.h +0 -2
index df9f0c7..7ed4d23 100644
@@ -25,11 +25,9 @@
  */
 typedef struct gpiod_chip gpiod_chip_struct;
 typedef struct gpiod_line_bulk gpiod_line_bulk_struct;
-typedef struct gpiod_chip_iter gpiod_chip_iter_struct;
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(gpiod_chip_struct, gpiod_chip_close);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(gpiod_line_bulk_struct, gpiod_line_bulk_free);
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(gpiod_chip_iter_struct, gpiod_chip_iter_free);
 
 /* These are private definitions and should not be used directly. */
 typedef void (*_gpiod_test_func)(void);
tests/tests-iter.c deleted +0 -100

100 lines not on this page · raw message

tools/gpiodetect.c +22 -7
index 1c992a4..8e067f7 100644
@@ -5,6 +5,8 @@
  * Copyright (C) 2017-2018 Bartosz Golaszewski <[email protected]>
  */
 
+#include <dirent.h>
+#include <errno.h>
 #include <getopt.h>
 #include <gpiod.h>
 #include <stdio.h>
@@ -33,9 +35,9 @@ static void print_help(void)
 
 int main(int argc, char **argv)
 {
-	struct gpiod_chip_iter *iter;
+	int optc, opti, num_chips, i;
 	struct gpiod_chip *chip;
-	int optc, opti;
+	struct dirent **entries;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -62,18 +64,31 @@ int main(int argc, char **argv)
 	if (argc > 0)
 		die("unrecognized argument: %s", argv[0]);
 
-	iter = gpiod_chip_iter_new();
-	if (!iter)
-		die_perror("unable to access GPIO chips");
+	num_chips = scandir("/dev/", &entries, chip_dir_filter, alphasort);
+	if (num_chips < 0)
+		die_perror("unable to scan /dev");
+
+	for (i = 0; i < num_chips; i++) {
+		chip = gpiod_chip_open_by_name(entries[i]->d_name);
+		if (!chip) {
+			if (errno == EACCES)
+				printf("%s Permission denied\n",
+				       entries[i]->d_name);
+			else
+				die_perror("unable to open %s",
+					   entries[i]->d_name);
+		}
 
-	gpiod_foreach_chip(iter, chip) {
 		printf("%s [%s] (%u lines)\n",
 		       gpiod_chip_name(chip),
 		       gpiod_chip_label(chip),
 		       gpiod_chip_num_lines(chip));
+
+		gpiod_chip_close(chip);
+		free(entries[i]);
 	}
 
-	gpiod_chip_iter_free(iter);
+	free(entries);
 
 	return EXIT_SUCCESS;
 }
tools/gpiofind.c +16 -11
index ffb8fc0..4acf621 100644
@@ -5,6 +5,7 @@
  * Copyright (C) 2017-2018 Bartosz Golaszewski <[email protected]>
  */
 
+#include <dirent.h>
 #include <errno.h>
 #include <getopt.h>
 #include <gpiod.h>
@@ -34,10 +35,10 @@ static void print_help(void)
 
 int main(int argc, char **argv)
 {
-	struct gpiod_chip_iter *iter;
+	int i, num_chips, optc, opti;
 	struct gpiod_chip *chip;
 	struct gpiod_line *line;
-	int optc, opti;
+	struct dirent **entries;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -64,23 +65,27 @@ int main(int argc, char **argv)
 	if (argc != 1)
 		die("exactly one GPIO line name must be specified");
 
-	iter = gpiod_chip_iter_new();
-	if (!iter)
-		die_perror("unable to access GPIO chips");
+	num_chips = scandir("/dev/", &entries, chip_dir_filter, alphasort);
+	if (num_chips < 0)
+		die_perror("unable to scan /dev");
+
+	for (i = 0; i < num_chips; i++) {
+		chip = gpiod_chip_open_by_name(entries[i]->d_name);
+		if (!chip) {
+			if (errno == EACCES)
+				continue;
+
+			die_perror("unable to open %s", entries[i]->d_name);
+		}
 
-	gpiod_foreach_chip(iter, chip) {
 		line = gpiod_chip_find_line_unique(chip, argv[0]);
 		if (line) {
 			printf("%s %u\n",
 			       gpiod_chip_name(chip), gpiod_line_offset(line));
-			gpiod_chip_iter_free(iter);
+			gpiod_chip_close(chip);
 			return EXIT_SUCCESS;
 		}
-
-		if (errno != ENOENT)
-			die_perror("error performing the line lookup");
 	}
 
-	gpiod_chip_iter_free(iter);
 	return EXIT_FAILURE;
 }
tools/gpioinfo.c +20 -7
index dd4a388..e3dbde7 100644
@@ -5,6 +5,7 @@
  * Copyright (C) 2017-2018 Bartosz Golaszewski <[email protected]>
  */
 
+#include <dirent.h>
 #include <errno.h>
 #include <getopt.h>
 #include <gpiod.h>
@@ -180,9 +181,9 @@ static void list_lines(struct gpiod_chip *chip)
 
 int main(int argc, char **argv)
 {
-	struct gpiod_chip_iter *chip_iter;
+	int num_chips, i, optc, opti;
 	struct gpiod_chip *chip;
-	int i, optc, opti;
+	struct dirent **entries;
 
 	for (;;) {
 		optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -207,14 +208,26 @@ int main(int argc, char **argv)
 	argv += optind;
 
 	if (argc == 0) {
-		chip_iter = gpiod_chip_iter_new();
-		if (!chip_iter)
-			die_perror("error accessing GPIO chips");
+		num_chips = scandir("/dev/", &entries,
+				    chip_dir_filter, alphasort);
+		if (num_chips < 0)
+			die_perror("unable to scan /dev");
+
+		for (i = 0; i < num_chips; i++) {
+			chip = gpiod_chip_open_by_name(entries[i]->d_name);
+			if (!chip) {
+				if (errno == EACCES)
+					printf("%s Permission denied\n",
+					       entries[i]->d_name);
+				else
+					die_perror("unable to open %s",
+						   entries[i]->d_name);
+			}
 
-		gpiod_foreach_chip(chip_iter, chip)
 			list_lines(chip);
 
-		gpiod_chip_iter_free(chip_iter);
+			gpiod_chip_close(chip);
+		}
 	} else {
 		for (i = 0; i < argc; i++) {
 			chip = gpiod_chip_open_lookup(argv[i]);
tools/tools-common.c +15 -0
index af05102..b64ef93 100644
@@ -101,3 +101,18 @@ int make_signalfd(void)
 
 	return sigfd;
 }
+
+int chip_dir_filter(const struct dirent *entry)
+{
+	bool is_chip;
+	char *path;
+	int ret;
+
+	ret = asprintf(&path, "/dev/%s", entry->d_name);
+	if (ret < 0)
+		return 0;
+
+	is_chip = gpiod_is_gpiochip_device(path);
+	free(path);
+	return !!is_chip;
+}
tools/tools-common.h +3 -0
index 8ccde62..4148dd8 100644
@@ -8,6 +8,8 @@
 #ifndef __GPIOD_TOOLS_COMMON_H__
 #define __GPIOD_TOOLS_COMMON_H__
 
+#include <dirent.h>
+
 /*
  * Various helpers for the GPIO tools.
  *
@@ -29,5 +31,6 @@ void print_version(void);
 int bias_flags(const char *option);
 void print_bias_help(void);
 int make_signalfd(void);
+int chip_dir_filter(const struct dirent *entry);
 
 #endif /* __GPIOD_TOOLS_COMMON_H__ */
Signature · 1 line
2.29.1
Reviewed-by on the v1 series
#15 lore raw
In reply to Bartosz Golaszewski
Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

The following series removes a lot of interfaces that were deemed overkill in libgpiod and the removal of which was suggested to me before proceeding with the new API.

This leaves a couple holes in the library but we'll follow them up with more improvements all over the tree. We'll create a new object called gpiod_request for dealing with line requests of arbitrary size. We'll probably remove the the bulk objects from bindings and eventually we'll switch to using the v2 kernel uAPI.

Quoted 3 lines

Andy - a note for you: I know you're always very thorough in your reviews but in this case let's consider this series preparing a construction zone for the new API. Please don't nitpick too much. :)

I don't know what you are talking about. The series looks nice, esp. taking into account statistics! FWIW,

One side note, though. Are you already plan to support autotools-2.70?

Quoted 60 lines

Bartosz Golaszewski (14):
bindings: cxx: check for error from gpiod_line_bulk_new()
build: drop the message about tests having been built successfully
core: export gpiod_is_gpiochip_device()
bulk: drop the limit on the max number of lines
core: drop line iterators
treewide: kill opening chips by label
API: move gpiod_line_get_chip() to line attributes section
core: kill gpiod_line_close_chip()
core: kill gpiod_line_get()
treewide: kill global line lookup
treewide: kill find_lines()
core: rework gpiod_chip_find_line()
build: add a configure switch for building examples
core: kill chip iterators

 bindings/cxx/Makefile.am                |   8 +-
 bindings/cxx/chip.cpp                   |  47 +--
 bindings/cxx/examples/Makefile.am       |   4 +-
 bindings/cxx/examples/gpiodetectcxx.cpp |  13 +-
 bindings/cxx/examples/gpiofindcxx.cpp   |  18 +-
 bindings/cxx/examples/gpioinfocxx.cpp   |  43 ++-
 bindings/cxx/gpiod.hpp                  | 147 +-------
 bindings/cxx/iter.cpp                   |  95 +----
 bindings/cxx/line.cpp                   |  15 -
 bindings/cxx/line_bulk.cpp              |  24 +-
 bindings/cxx/tests/tests-chip.cpp       |  62 ++--
 bindings/cxx/tests/tests-iter.cpp       |  37 --
 bindings/cxx/tests/tests-line.cpp       |  19 -
 bindings/python/Makefile.am             |  10 +-
 bindings/python/examples/gpiodetect.py  |  12 +-
 bindings/python/examples/gpiofind.py    |  15 +-
 bindings/python/examples/gpioinfo.py    |  33 +-
 bindings/python/gpiodmodule.c           | 473 +++++++-----------------
 bindings/python/tests/gpiod_py_test.py  | 100 +----
 configure.ac                            |  12 +
 include/gpiod.h                         | 258 ++-----------
 lib/Makefile.am                         |   2 +-
 lib/core.c                              |  12 +-
 lib/helpers.c                           | 176 +++------
 lib/iter.c                              | 171 ---------
 tests/Makefile.am                       |  13 -
 tests/gpiod-test.h                      |   4 -
 tests/tests-bulk.c                      |   9 -
 tests/tests-chip.c                      |  97 +----
 tests/tests-iter.c                      | 123 ------
 tests/tests-line.c                      |  52 ---
 tools/gpiodetect.c                      |  29 +-
 tools/gpiofind.c                        |  34 +-
 tools/gpioinfo.c                        |  41 +-
 tools/gpiomon.c                         |   5 +-
 tools/tools-common.c                    |  15 +
 tools/tools-common.h                    |   3 +
 37 files changed, 557 insertions(+), 1674 deletions(-)
 delete mode 100644 lib/iter.c
 delete mode 100644 tests/tests-iter.c
Signature · 1 line
2.29.1
Signature · 2 lines
With Best Regards,
Andy Shevchenko
#16 lore raw
Quoted 18 lines · Andy Shevchenko
Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

The following series removes a lot of interfaces that were deemed overkill in libgpiod and the removal of which was suggested to me before proceeding with the new API.

This leaves a couple holes in the library but we'll follow them up with more improvements all over the tree. We'll create a new object called gpiod_request for dealing with line requests of arbitrary size. We'll probably remove the the bulk objects from bindings and eventually we'll switch to using the v2 kernel uAPI.

Quoted 3 lines

Andy - a note for you: I know you're always very thorough in your reviews but in this case let's consider this series preparing a construction zone for the new API. Please don't nitpick too much. :)

Quoted 7 lines

I don't know what you are talking about. The series looks nice, esp. taking into account statistics! FWIW,

One side note, though. Are you already plan to support autotools-2.70?

Isn't it already supported? 2.69 is the minimum version, 2.70 should just work.

Bartosz

#17 lore raw
Quoted 23 lines · Bartosz Golaszewski
Quoted 21 lines · Andy Shevchenko
Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

The following series removes a lot of interfaces that were deemed overkill in libgpiod and the removal of which was suggested to me before proceeding with the new API.

This leaves a couple holes in the library but we'll follow them up with more improvements all over the tree. We'll create a new object called gpiod_request for dealing with line requests of arbitrary size. We'll probably remove the the bulk objects from bindings and eventually we'll switch to using the v2 kernel uAPI.

Quoted 3 lines

Andy - a note for you: I know you're always very thorough in your reviews but in this case let's consider this series preparing a construction zone for the new API. Please don't nitpick too much. :)

I don't know what you are talking about. The series looks nice, esp. taking into account statistics! FWIW,

Quoted 6 lines
Quoted 4 lines

One side note, though. Are you already plan to support autotools-2.70?

Isn't it already supported? 2.69 is the minimum version, 2.70 should just work.

Have you read an article on LWN about changes [1]? There are a lot of incompatibilities (note between 2.69 and 2.70 _8_ years passed).

[1]: https://lwn.net/Articles/839395/

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#18 lore raw
Quoted 32 lines · Andy Shevchenko
Quoted 23 lines · Bartosz Golaszewski
Quoted 21 lines · Andy Shevchenko
Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

The following series removes a lot of interfaces that were deemed overkill in libgpiod and the removal of which was suggested to me before proceeding with the new API.

This leaves a couple holes in the library but we'll follow them up with more improvements all over the tree. We'll create a new object called gpiod_request for dealing with line requests of arbitrary size. We'll probably remove the the bulk objects from bindings and eventually we'll switch to using the v2 kernel uAPI.

Quoted 3 lines

Andy - a note for you: I know you're always very thorough in your reviews but in this case let's consider this series preparing a construction zone for the new API. Please don't nitpick too much. :)

I don't know what you are talking about. The series looks nice, esp. taking into account statistics! FWIW,

Quoted 6 lines
Quoted 4 lines

One side note, though. Are you already plan to support autotools-2.70?

Isn't it already supported? 2.69 is the minimum version, 2.70 should just work.

Quoted 10 lines

Have you read an article on LWN about changes [1]? There are a lot of incompatibilities (note between 2.69 and 2.70 _8_ years passed).

[1]: https://lwn.net/Articles/839395/

--
With Best Regards,
Andy Shevchenko

How will distros handle the incompatibilities? I don't think all projects - many no longer even supported - will suddenly switch to new autoconf.

Bartosz

#19 lore raw
Quoted 6 lines · Bartosz Golaszewski
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · Bartosz Golaszewski
Quoted 1 line · Andy Shevchenko

On Thu, Dec 10, 2020 at 02:23:01PM +0100, Bartosz Golaszewski wrote:

...

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

One side note, though. Are you already plan to support autotools-2.70?

Isn't it already supported? 2.69 is the minimum version, 2.70 should just work.

Quoted 8 lines
Quoted 4 lines

Have you read an article on LWN about changes [1]? There are a lot of incompatibilities (note between 2.69 and 2.70 _8_ years passed).

[1]: https://lwn.net/Articles/839395/

How will distros handle the incompatibilities? I don't think all projects - many no longer even supported - will suddenly switch to new autoconf.

It's PITA for distributions. Last time I remember that nice jump from 2.13 to 2.50. However, 2.69 should be closer to 2.70. I believe the distributions will patch all broken packages one-by-one. I think there is no need to keep two autoconf packages together in this case, but who knows, it might be a last resort.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#20 lore raw
Quoted 24 lines · Andy Shevchenko
Quoted 6 lines · Bartosz Golaszewski
Quoted 4 lines · Andy Shevchenko
Quoted 3 lines · Bartosz Golaszewski
Quoted 1 line · Andy Shevchenko

On Thu, Dec 10, 2020 at 02:23:01PM +0100, Bartosz Golaszewski wrote:

...

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Quoted 1 line

One side note, though. Are you already plan to support autotools-2.70?

Isn't it already supported? 2.69 is the minimum version, 2.70 should just work.

Quoted 8 lines
Quoted 4 lines

Have you read an article on LWN about changes [1]? There are a lot of incompatibilities (note between 2.69 and 2.70 _8_ years passed).

[1]: https://lwn.net/Articles/839395/

How will distros handle the incompatibilities? I don't think all projects - many no longer even supported - will suddenly switch to new autoconf.

Quoted 10 lines

It's PITA for distributions. Last time I remember that nice jump from 2.13 to 2.50. However, 2.69 should be closer to 2.70. I believe the distributions will patch all broken packages one-by-one. I think there is no need to keep two autoconf packages together in this case, but who knows, it might be a last resort.

--
With Best Regards,
Andy Shevchenko

Ok, so responding to your question: I'll upgrade to 2.70 once it's necessary. :)

Bart

#21 lore raw
In reply to Andy Shevchenko
Quoted 14 lines · Andy Shevchenko
Quoted 12 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

The following series removes a lot of interfaces that were deemed overkill in libgpiod and the removal of which was suggested to me before proceeding with the new API.

This leaves a couple holes in the library but we'll follow them up with more improvements all over the tree. We'll create a new object called gpiod_request for dealing with line requests of arbitrary size. We'll probably remove the the bulk objects from bindings and eventually we'll switch to using the v2 kernel uAPI.

Quoted 9 lines
Quoted 3 lines

Andy - a note for you: I know you're always very thorough in your reviews but in this case let's consider this series preparing a construction zone for the new API. Please don't nitpick too much. :)

I don't know what you are talking about. The series looks nice, esp. taking into account statistics! FWIW,

I applied the series so that we can move on. Since we're now getting per-chip unique lines, we'll probably have to redesign line lookup again but that's alright.

Bartosz