iter: rework chip iterators

We can significantly simplify the chip iterators by performing all
actions that can yield an error in gpiod_chip_iter_new(). Instead of
opening each subsequent chip in gpiod_chip_iter_next(), open them all
in the new function and make next just return an active handle.

Fix relevant test cases and macros, remove related routines that are
no longer needed.

Signed-off-by: Bartosz Golaszewski <bartekgola@gmail.com>

diff --git a/include/gpiod.h b/include/gpiod.h
index 03c5b2398228c3ed0bd5f1477ec7f275f98d5d5d..3276036181df3a854ab2b84e5f09cc4643ec8c30 100644 (file)
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -1153,8 +1153,9 @@ struct gpiod_chip * gpiod_line_get_chip(struct gpiod_line *line) GPIOD_API;
  * @brief Create a new gpiochip iterator.
  * @return Pointer to a new chip iterator object or NULL if an error occurred.
  *
- * Internally this routine scand the /dev/ directory for GPIO chip device
- * files and stores their list in the iterator structure.
+ * Internally this routine scans the /dev/ directory for GPIO chip device
+ * files, opens them and stores their the handles until ::gpiod_chip_iter_free
+ * or ::gpiod_chip_iter_free_noclose is called.
  */
 struct gpiod_chip_iter * gpiod_chip_iter_new(void) GPIOD_API;
 
@@ -1181,9 +1182,7 @@ void gpiod_chip_iter_free_noclose(struct gpiod_chip_iter *iter) GPIOD_API;
  * @param iter The gpiochip iterator object.
  * @return Pointer to an open gpiochip handle or NULL if the next chip can't
  *         be accessed.
- *
- * Internally this routine tries to open the next /dev/gpiochipX device file.
- * If an error occurs or no more chips are present, the function returns NULL.
+ * @note The previous chip handle will be closed using ::gpiod_chip_iter_free.
  */
 struct gpiod_chip *
 gpiod_chip_iter_next(struct gpiod_chip_iter *iter) GPIOD_API;
@@ -1212,11 +1211,11 @@ gpiod_chip_iter_next_noclose(struct gpiod_chip_iter *iter) GPIOD_API;
  */
 #define gpiod_foreach_chip(iter, chip)                                 \
        for ((chip) = gpiod_chip_iter_next(iter);                       \
-            !gpiod_chip_iter_done(iter);                               \
+            (chip);                                                    \
             (chip) = gpiod_chip_iter_next(iter))
 
 /**
- * @brief Iterate over all gpiochip present in the system without closing them.
+ * @brief Iterate over all chips present in the system without closing them.
  * @param iter An initialized GPIO chip iterator.
  * @param chip Pointer to a GPIO chip handle. On each iteration the newly
  *             opened chip handle is assigned to this argument.
@@ -1227,37 +1226,9 @@ gpiod_chip_iter_next_noclose(struct gpiod_chip_iter *iter) GPIOD_API;
  */
 #define gpiod_foreach_chip_noclose(iter, chip)                         \
        for ((chip) = gpiod_chip_iter_next_noclose(iter);               \
-            !gpiod_chip_iter_done(iter);                               \
+            (chip);                                                    \
             (chip) = gpiod_chip_iter_next_noclose(iter))
 
-/**
- * @brief Check if we're done iterating over gpiochips on this iterator.
- * @param iter The gpiochip iterator object.
- * @return True if we've iterated over all chips, false otherwise.
- */
-bool gpiod_chip_iter_done(struct gpiod_chip_iter *iter) GPIOD_API;
-
-/**
- * @brief Check if we've encountered an error condition while opening a
- *        gpiochip.
- * @param iter The gpiochip iterator object.
- * @return True if there was an error opening a gpiochip device file,
- *         false otherwise.
- */
-bool gpiod_chip_iter_err(struct gpiod_chip_iter *iter) GPIOD_API;
-
-/**
- * @brief Get the name of the gpiochip that we failed to access.
- * @param iter The gpiochip iterator object.
- * @return If ::gpiod_chip_iter_iserr returned true, this function returns a
- *         pointer to the name of the gpiochip that we failed to access.
- *         If there was no error this function returns NULL.
- * @note This function will return NULL if the internal memory
- *       allocation fails.
- */
-const char *
-gpiod_chip_iter_failed_chip(struct gpiod_chip_iter *iter) GPIOD_API;
-
 /**
  * @brief Possible states of a line iterator.
  */

diff --git a/src/lib/helpers.c b/src/lib/helpers.c
index fc85b5851c7c57b346dc35cbd3d3ff90f1830e73..085f44efb61a020732a7384374157763a3de6d9d 100644 (file)
--- a/src/lib/helpers.c
+++ b/src/lib/helpers.c
@@ -70,9 +70,6 @@ struct gpiod_chip * gpiod_chip_open_by_label(const char *label)
                return NULL;
 
        gpiod_foreach_chip(iter, chip) {
-               if (gpiod_chip_iter_err(iter))
-                       goto out;
-
                if (strcmp(label, gpiod_chip_label(chip)) == 0) {
                        gpiod_chip_iter_free_noclose(iter);
                        return chip;
@@ -80,8 +77,6 @@ struct gpiod_chip * gpiod_chip_open_by_label(const char *label)
        }
 
        errno = ENOENT;
-
-out:
        gpiod_chip_iter_free(iter);
 
        return NULL;
@@ -365,9 +360,6 @@ struct gpiod_line * gpiod_line_find(const char *name)
                return NULL;
 
        gpiod_foreach_chip(iter, chip) {
-               if (gpiod_chip_iter_err(iter))
-                       goto out;
-
                line = gpiod_chip_find_line(chip, name);
                if (line) {
                        gpiod_chip_iter_free_noclose(iter);

diff --git a/src/lib/iter.c b/src/lib/iter.c
index 1362932b0b28b37af828718a3ea445c87bfc3751..c6fdbaf55d071d3e7ea6188f46ad4a50d4b1e75b 100644 (file)
--- a/src/lib/iter.c
+++ b/src/lib/iter.c
@@ -15,21 +15,10 @@
 #include <string.h>
 #include <dirent.h>
 
-enum {
-       CHIP_ITER_INIT = 0,
-       CHIP_ITER_DONE,
-       CHIP_ITER_ERR,
-};
-
 struct gpiod_chip_iter {
-       struct dirent **dirs;
-       unsigned int num_dirs;
-
+       struct gpiod_chip **chips;
+       unsigned int num_chips;
        unsigned int offset;
-       int state;
-       struct gpiod_chip *current;
-
-       char *failed_chip;
 };
 
 static int dir_filter(const struct dirent *dir)
@@ -37,35 +26,70 @@ static int dir_filter(const struct dirent *dir)
        return !strncmp(dir->d_name, "gpiochip", 8);
 }
 
+static void free_dirs(struct dirent ***dirs, unsigned int num_dirs)
+{
+       unsigned int i;
+
+       for (i = 0; i < num_dirs; i++)
+               free((*dirs)[i]);
+       free(*dirs);
+}
+
 struct gpiod_chip_iter * gpiod_chip_iter_new(void)
 {
-       struct gpiod_chip_iter *new;
-       int rv;
+       struct gpiod_chip_iter *iter;
+       struct dirent **dirs;
+       int i, num_chips;
 
-       new = malloc(sizeof(*new));
-       if (!new)
+       num_chips = scandir("/dev", &dirs, dir_filter, alphasort);
+       if (num_chips < 0)
                return NULL;
 
-       memset(new, 0, sizeof(*new));
+       iter = malloc(sizeof(*iter));
+       if (!iter)
+               goto err_free_dirs;
 
-       rv = scandir("/dev", &new->dirs, dir_filter, alphasort);
-       if (rv < 0) {
-               free(new);
-               return NULL;
+       iter->num_chips = num_chips;
+       iter->offset = 0;
+
+       if (num_chips == 0)
+               return iter;
+
+       iter->chips = calloc(num_chips, sizeof(*iter->chips));
+       if (!iter->chips)
+               goto err_free_iter;
+
+       for (i = 0; i < num_chips; i++) {
+               iter->chips[i] = gpiod_chip_open_by_name(dirs[i]->d_name);
+               if (!iter->chips[i])
+                       goto err_close_chips;
        }
 
-       new->num_dirs = rv;
-       new->offset = 0;
-       new->state = CHIP_ITER_INIT;
+       free_dirs(&dirs, num_chips);
+
+       return iter;
+
+err_close_chips:
+       for (i = 0; i < num_chips; i++) {
+               if (iter->chips[i])
+                       gpiod_chip_close(iter->chips[i]);
+       }
+
+       free(iter->chips);
+
+err_free_iter:
+       free(iter);
+
+err_free_dirs:
+       free_dirs(&dirs, num_chips);
 
-       return new;
+       return NULL;
 }
 
 void gpiod_chip_iter_free(struct gpiod_chip_iter *iter)
 {
-       if (iter->current)
-               gpiod_chip_close(iter->current);
-
+       if (iter->offset > 0 && iter->offset < iter->num_chips)
+               gpiod_chip_close(iter->chips[iter->offset - 1]);
        gpiod_chip_iter_free_noclose(iter);
 }
 
@@ -73,21 +97,20 @@ void gpiod_chip_iter_free_noclose(struct gpiod_chip_iter *iter)
 {
        unsigned int i;
 
-       if (iter->failed_chip)
-               free(iter->failed_chip);
-
-       for (i = 0; i < iter->num_dirs; i++)
-               free(iter->dirs[i]);
+       for (i = iter->offset; i < iter->num_chips; i++) {
+               if (iter->chips[i])
+                       gpiod_chip_close(iter->chips[i]);
+       }
 
-       free(iter->dirs);
+       free(iter->chips);
        free(iter);
 }
 
 struct gpiod_chip * gpiod_chip_iter_next(struct gpiod_chip_iter *iter)
 {
-       if (iter->current) {
-               gpiod_chip_close(iter->current);
-               iter->current = NULL;
+       if (iter->offset > 0) {
+               gpiod_chip_close(iter->chips[iter->offset - 1]);
+               iter->chips[iter->offset - 1] = NULL;
        }
 
        return gpiod_chip_iter_next_noclose(iter);
@@ -95,47 +118,8 @@ struct gpiod_chip * gpiod_chip_iter_next(struct gpiod_chip_iter *iter)
 
 struct gpiod_chip * gpiod_chip_iter_next_noclose(struct gpiod_chip_iter *iter)
 {
-       struct gpiod_chip *chip;
-       const char *dev;
-
-       if (iter->offset >= iter->num_dirs) {
-               iter->state = CHIP_ITER_DONE;
-               return NULL;
-       }
-
-       iter->state = CHIP_ITER_INIT;
-       if (iter->failed_chip) {
-               free(iter->failed_chip);
-               iter->failed_chip = NULL;
-       }
-
-       dev = iter->dirs[iter->offset++]->d_name;
-
-       chip = gpiod_chip_open_by_name(dev);
-       if (!chip) {
-               iter->state = CHIP_ITER_ERR;
-               iter->failed_chip = strdup(dev);
-               /* No point in an error check here. */
-       }
-
-       iter->current = chip;
-       return iter->current;
-}
-
-bool gpiod_chip_iter_done(struct gpiod_chip_iter *iter)
-{
-       return iter->state == CHIP_ITER_DONE;
-}
-
-bool gpiod_chip_iter_err(struct gpiod_chip_iter *iter)
-{
-       return iter->state == CHIP_ITER_ERR;
-}
-
-const char *
-gpiod_chip_iter_failed_chip(struct gpiod_chip_iter *iter)
-{
-       return iter->failed_chip;
+       return iter->offset < (iter->num_chips)
+                                       ? iter->chips[iter->offset++] : NULL;
 }
 
 struct gpiod_line * gpiod_line_iter_next(struct gpiod_line_iter *iter)

diff --git a/src/tools/gpiodetect.c b/src/tools/gpiodetect.c
index 94163c6544962a2edbb520e1a17457fc72abcff1..3ba5f7f94a2663a1533986d3d4452e8c18643722 100644 (file)
--- a/src/tools/gpiodetect.c
+++ b/src/tools/gpiodetect.c
@@ -68,10 +68,6 @@ int main(int argc, char **argv)
                die_perror("unable to access GPIO chips");
 
        gpiod_foreach_chip(iter, chip) {
-               if (gpiod_chip_iter_err(iter))
-                       die_perror("error accessing gpiochip %s",
-                                  gpiod_chip_iter_failed_chip(iter));
-
                printf("%s [%s] (%u lines)\n",
                       gpiod_chip_name(chip),
                       gpiod_chip_label(chip),

diff --git a/src/tools/gpioinfo.c b/src/tools/gpioinfo.c
index 0dda38ce49eb5849ed9d1193a6f1f3c31036875b..9bf940cee12e6ee26604b60c9da4365cec6ec90c 100644 (file)
--- a/src/tools/gpioinfo.c
+++ b/src/tools/gpioinfo.c
@@ -182,13 +182,8 @@ int main(int argc, char **argv)
                if (!chip_iter)
                        die_perror("error accessing GPIO chips");
 
-               gpiod_foreach_chip(chip_iter, chip) {
-                       if (gpiod_chip_iter_err(chip_iter))
-                               die_perror("error accessing gpiochip %s",
-                                   gpiod_chip_iter_failed_chip(chip_iter));
-
+               gpiod_foreach_chip(chip_iter, chip)
                        list_lines(chip);
-               }
 
                gpiod_chip_iter_free(chip_iter);
        } else {

diff --git a/tests/tests-iter.c b/tests/tests-iter.c
index 106fc8043e5ec7879c7d82b000308463f055a4d1..4555fa44a837afcacefdb95ca391f2f27fb17835 100644 (file)
--- a/tests/tests-iter.c
+++ b/tests/tests-iter.c
@@ -24,8 +24,6 @@ static void chip_iter(void)
        TEST_ASSERT_NOT_NULL(iter);
 
        gpiod_foreach_chip(iter, chip) {
-               TEST_ASSERT(!gpiod_chip_iter_err(iter));
-
                if (strcmp(gpiod_chip_label(chip), "gpio-mockup-A") == 0)
                        A = true;
                else if (strcmp(gpiod_chip_label(chip), "gpio-mockup-B") == 0)
@@ -58,8 +56,6 @@ static void chip_iter_noclose(void)
        TEST_ASSERT_NOT_NULL(iter);
 
        gpiod_foreach_chip_noclose(iter, chip) {
-               TEST_ASSERT(!gpiod_chip_iter_err(iter));
-
                if (strcmp(gpiod_chip_label(chip), "gpio-mockup-A") == 0) {
                        A = true;
                        chipA = chip;
@@ -100,8 +96,6 @@ static void chip_iter_break(void)
        TEST_ASSERT_NOT_NULL(iter);
 
        gpiod_foreach_chip(iter, chip) {
-               TEST_ASSERT(!gpiod_chip_iter_err(iter));
-
                if ((strcmp(gpiod_chip_label(chip), "gpio-mockup-A") == 0) ||
                    (strcmp(gpiod_chip_label(chip), "gpio-mockup-B") == 0) ||
                    (strcmp(gpiod_chip_label(chip), "gpio-mockup-C") == 0))