core: drop line iterators
authorBartosz Golaszewski <bgolaszewski@baylibre.com>
Wed, 25 Nov 2020 10:45:55 +0000 (11:45 +0100)
committerBartosz Golaszewski <bgolaszewski@baylibre.com>
Mon, 14 Dec 2020 14:56:43 +0000 (15:56 +0100)
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.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
bindings/cxx/gpiod.hpp
bindings/cxx/iter.cpp
bindings/python/gpiodmodule.c
include/gpiod.h
lib/helpers.c
lib/iter.c
tests/gpiod-test.h
tests/tests-iter.c
tools/gpioinfo.c

index 8c8e6c9256d5bbd3b31b17fd83d88dbd0a78f841..0f1d9b2d4a11d6894c1d65443cc455c3d03b9251 100644 (file)
@@ -1083,7 +1083,6 @@ public:
 
 private:
 
-       ::std::shared_ptr<::gpiod_line_iter> _m_iter;
        line _m_current;
 };
 
index 79859108104fb443be2f13e95f73e0ba8e6c5f27..15c3925107a6808511b38cd8e7d30e6398361c5b 100644 (file)
@@ -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;
 }
index b9b57703eb256733e29addd10d991f40d33c74eb..11d1407f005235a4a93583726dba35915d8e8424 100644 (file)
@@ -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);
 }
 
index 9ffb446b45f8cc0420a9f561e01824d924a0804b..b5965eda0f0cadd7d00d1c6b1ff252eb70c2a33f 100644 (file)
@@ -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))
-
 /**
  * @}
  *
index a343f71baaf4f399fbeba1a8bd0d34251dbc9e33..3b7428b3cb0cd54e2c5246f93b78bd389d7d77e3 100644 (file)
@@ -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;
 }
index bfd28523b8069855a5aae7b19841c4af2db52653..2ff767c36e11313318d20c3bc56b6acd19c6859e 100644 (file)
@@ -17,12 +17,6 @@ struct gpiod_chip_iter {
        unsigned int offset;
 };
 
-struct gpiod_line_iter {
-       struct gpiod_line **lines;
-       unsigned int num_lines;
-       unsigned int offset;
-};
-
 static int dir_filter(const struct dirent *dir)
 {
        return !strncmp(dir->d_name, "gpiochip", 8);
@@ -127,45 +121,3 @@ struct gpiod_chip *gpiod_chip_iter_next_noclose(struct gpiod_chip_iter *iter)
        return iter->offset < (iter->num_chips)
                                        ? iter->chips[iter->offset++] : NULL;
 }
-
-struct gpiod_line_iter *gpiod_line_iter_new(struct gpiod_chip *chip)
-{
-       struct gpiod_line_iter *iter;
-       unsigned int i;
-
-       iter = malloc(sizeof(*iter));
-       if (!iter)
-               return NULL;
-
-       iter->num_lines = gpiod_chip_num_lines(chip);
-       iter->offset = 0;
-
-       iter->lines = calloc(iter->num_lines, sizeof(*iter->lines));
-       if (!iter->lines) {
-               free(iter);
-               return NULL;
-       }
-
-       for (i = 0; i < iter->num_lines; i++) {
-               iter->lines[i] = gpiod_chip_get_line(chip, i);
-               if (!iter->lines[i]) {
-                       free(iter->lines);
-                       free(iter);
-                       return NULL;
-               }
-       }
-
-       return iter;
-}
-
-void gpiod_line_iter_free(struct gpiod_line_iter *iter)
-{
-       free(iter->lines);
-       free(iter);
-}
-
-struct gpiod_line *gpiod_line_iter_next(struct gpiod_line_iter *iter)
-{
-       return iter->offset < (iter->num_lines)
-                                       ? iter->lines[iter->offset++] : NULL;
-}
index a43109a70c0b7a1b9e38c0a5412e6343572a00c7..df9f0c7fc34736031b4f6cbf2b9b71912bd1ae28 100644 (file)
 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);
index 8deee8ef6172182a826589dd682244849c8af7ab..163a820d939e42be11a62b510f19ddad71857cc9 100644 (file)
@@ -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);
-}
index 67be379505217697b0f7e0c182dcd8b3c768ea15..dd4a388432397e707722842e0c1002c8c5385a91 100644 (file)
@@ -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)