iter: rework line iterator API
authorBartosz Golaszewski <bartekgola@gmail.com>
Sun, 5 Nov 2017 19:57:25 +0000 (20:57 +0100)
committerBartosz Golaszewski <bartekgola@gmail.com>
Sun, 5 Nov 2017 19:57:25 +0000 (20:57 +0100)
The changes roughly correspond with what we did for chip iterators.

Only perform operations that can fail in gpiod_line_iter_new(), so
that there's no need to check for errors after every call to
gpiod_line_iter_next().

Make gpiod_line_iter an opaque pointer and provide 'new' and 'free'
functions.

Update all users and relevant test cases. Extend the test API with
a new cleanup function.

Signed-off-by: Bartosz Golaszewski <bartekgola@gmail.com>
include/gpiod.h
src/lib/helpers.c
src/lib/iter.c
src/tools/gpioinfo.c
tests/gpiod-test.c
tests/gpiod-test.h
tests/tests-iter.c

index 8c0ec91b1450f9c806110b31984cd0f0d82be318..0039cdaf2044a5bc10c7f261039dac85b37ca5dd 100644 (file)
@@ -45,6 +45,7 @@ extern "C" {
 struct gpiod_chip;
 struct gpiod_line;
 struct gpiod_chip_iter;
+struct gpiod_line_iter;
 
 /**
  * @defgroup __common__ Common helper macros
@@ -1229,98 +1230,28 @@ gpiod_chip_iter_next_noclose(struct gpiod_chip_iter *iter) GPIOD_API;
             (chip) = gpiod_chip_iter_next_noclose(iter))
 
 /**
- * @brief Possible states of a line iterator.
+ * @brief Create a new line iterator.
+ * @param chip Active gpiochip handle over whose lines we want to iterate.
+ * @return New line iterator or NULL if an error occurred.
  */
-enum {
-       GPIOD_LINE_ITER_STATE_INIT = 0,
-       /**< Line iterator is initiated or iterating over lines. */
-       GPIOD_LINE_ITER_STATE_DONE,
-       /**< Line iterator is done with all lines on this chip. */
-       GPIOD_LINE_ITER_STATE_ERR,
-       /**< There was an error retrieving info for a line. */
-};
-
-/**
- * @brief GPIO line iterator structure.
- *
- * This structure is used in conjunction with gpiod_line_iter_next() to
- * iterate over all GPIO lines of a single GPIO chip.
- */
-struct gpiod_line_iter {
-       unsigned int offset;
-       /**< Current line offset. */
-       struct gpiod_chip *chip;
-       /**< GPIO chip whose line we're iterating over. */
-       int state;
-       /**< Current state of the iterator. */
-};
-
-/**
- * @brief Static initializer for line iterators.
- * @param chip The gpiochip object whose lines we want to iterate over.
- */
-#define GPIOD_LINE_ITER_INITIALIZER(chip)                              \
-       { 0, (chip), GPIOD_LINE_ITER_STATE_INIT }
+struct gpiod_line_iter *
+gpiod_line_iter_new(struct gpiod_chip *chip) GPIOD_API;
 
 /**
- * @brief Initialize a GPIO line iterator.
- * @param iter Pointer to a GPIO line iterator.
- * @param chip The gpiochip object whose lines we want to iterate over.
+ * @brief Free all resources associated with a GPIO line iterator.
+ * @param iter Line iterator object.
  */
-static inline void gpiod_line_iter_init(struct gpiod_line_iter *iter,
-                                       struct gpiod_chip *chip)
-{
-       iter->offset = 0;
-       iter->chip = chip;
-       iter->state = GPIOD_LINE_ITER_STATE_INIT;
-}
+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 no more lines or
- *         and error occured.
+ * @return Pointer to the next GPIO line handle or NULL there are no more
+ *         lines left.
  */
 struct gpiod_line *
 gpiod_line_iter_next(struct gpiod_line_iter *iter) GPIOD_API;
 
-/**
- * @brief Check if we're done iterating over lines on this iterator.
- * @param iter The GPIO line iterator object.
- * @return True if we've iterated over all lines, false otherwise.
- */
-static inline bool gpiod_line_iter_done(const struct gpiod_line_iter *iter)
-{
-       return iter->state == GPIOD_LINE_ITER_STATE_DONE;
-}
-
-/**
- * @brief Check if we've encountered an error condition while retrieving
- *        info for a line.
- * @param iter The GPIO line iterator object.
- * @return True if there was an error retrieving info about a GPIO line,
- *         false otherwise.
- */
-static inline bool gpiod_line_iter_err(const struct gpiod_line_iter *iter)
-{
-       return iter->state == GPIOD_LINE_ITER_STATE_ERR;
-}
-
-/**
- * @brief Get the offset of the last line we tried to open.
- * @param iter The GPIO line iterator object.
- * @return The offset of the last line we tried to open - whether we failed
- *         or succeeded to do so.
- *
- * If this function is called before gpiod_line_iter_next() is called at least
- * once, the results are undefined.
- */
-static inline unsigned int
-gpiod_line_iter_last_offset(const struct gpiod_line_iter *iter)
-{
-       return iter->offset - 1;
-}
-
 /**
  * @brief Iterate over all GPIO lines of a single chip.
  * @param iter An initialized GPIO line iterator.
@@ -1329,7 +1260,7 @@ gpiod_line_iter_last_offset(const struct gpiod_line_iter *iter)
  */
 #define gpiod_foreach_line(iter, line)                                 \
        for ((line) = gpiod_line_iter_next(iter);                       \
-            !gpiod_line_iter_done(iter);                               \
+            (line);                                                    \
             (line) = gpiod_line_iter_next(iter))
 
 /**
index 085f44efb61a020732a7384374157763a3de6d9d..3b0bcf923765ab6849e94794f07c55562f07a359 100644 (file)
@@ -104,21 +104,24 @@ struct gpiod_chip * gpiod_chip_open_lookup(const char *descr)
 struct gpiod_line *
 gpiod_chip_find_line(struct gpiod_chip *chip, const char *name)
 {
-       struct gpiod_line_iter iter;
+       struct gpiod_line_iter *iter;
        struct gpiod_line *line;
        const char *tmp;
 
-       gpiod_line_iter_init(&iter, chip);
-       gpiod_foreach_line(&iter, line) {
-               if (gpiod_line_iter_err(&iter))
-                       return NULL;
+       iter = gpiod_line_iter_new(chip);
+       if (!iter)
+               return NULL;
 
+       gpiod_foreach_line(iter, line) {
                tmp = gpiod_line_name(line);
-               if (tmp && strcmp(tmp, name) == 0)
+               if (tmp && strcmp(tmp, name) == 0) {
+                       gpiod_line_iter_free(iter);
                        return line;
+               }
        }
 
        errno = ENOENT;
+       gpiod_line_iter_free(iter);
 
        return NULL;
 }
index c6fdbaf55d071d3e7ea6188f46ad4a50d4b1e75b..62a416554083714cf7827bbd7912242ae458a983 100644 (file)
@@ -21,6 +21,12 @@ 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);
@@ -122,19 +128,44 @@ struct gpiod_chip * gpiod_chip_iter_next_noclose(struct gpiod_chip_iter *iter)
                                        ? iter->chips[iter->offset++] : NULL;
 }
 
-struct gpiod_line * gpiod_line_iter_next(struct gpiod_line_iter *iter)
+struct gpiod_line_iter * gpiod_line_iter_new(struct gpiod_chip *chip)
 {
-       struct gpiod_line *line;
+       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;
 
-       if (iter->offset >= gpiod_chip_num_lines(iter->chip)) {
-               iter->state = GPIOD_LINE_ITER_STATE_DONE;
+       iter->lines = calloc(iter->num_lines, sizeof(*iter->lines));
+       if (!iter->lines) {
+               free(iter);
                return NULL;
        }
 
-       iter->state = GPIOD_LINE_ITER_STATE_INIT;
-       line = gpiod_chip_get_line(iter->chip, iter->offset++);
-       if (!line)
-               iter->state = GPIOD_LINE_ITER_STATE_ERR;
+       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);
+}
 
-       return line;
+struct gpiod_line * gpiod_line_iter_next(struct gpiod_line_iter *iter)
+{
+       return iter->offset < (iter->num_lines)
+                                       ? iter->lines[iter->offset++] : NULL;
 }
index 9bf940cee12e6ee26604b60c9da4365cec6ec90c..4a13ec1ef74a19821ebc36a07146db58c57c6a77 100644 (file)
@@ -88,22 +88,21 @@ 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;
-       struct gpiod_line_iter iter;
        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_line_iter_init(&iter, chip);
-       gpiod_foreach_line(&iter, line) {
-               if (gpiod_line_iter_err(&iter))
-                       die_perror("error retrieving info for line %u",
-                                  gpiod_line_iter_last_offset(&iter));
-
+       gpiod_foreach_line(iter, line) {
                offset = gpiod_line_offset(line);
                name = gpiod_line_name(line);
                consumer = gpiod_line_consumer(line);
@@ -147,6 +146,8 @@ static void list_lines(struct gpiod_chip *chip)
 
                printf("\n");
        }
+
+       gpiod_line_iter_free(iter);
 }
 
 int main(int argc, char **argv)
index d64a24db558a72cb98da95142ed051c755d340e1..0c68e8ccd559baceb2da7d6ecb4c262a1105916e 100644 (file)
@@ -1043,6 +1043,12 @@ void test_free_chip_iter(struct gpiod_chip_iter **iter)
                gpiod_chip_iter_free(*iter);
 }
 
+void test_free_line_iter(struct gpiod_line_iter **iter)
+{
+       if (*iter)
+               gpiod_line_iter_free(*iter);
+}
+
 void test_free_chip_iter_noclose(struct gpiod_chip_iter **iter)
 {
        if (*iter)
index 0f451882851aaec37e95b7b57cd4b4be8be8dc87..44cabffd3ca88898fdd8b2105e1d6520751b4793 100644 (file)
@@ -130,6 +130,7 @@ void test_line_close_chip(struct gpiod_line **line);
 void test_free_str(char **str);
 void test_free_chip_iter(struct gpiod_chip_iter **iter);
 void test_free_chip_iter_noclose(struct gpiod_chip_iter **iter);
+void test_free_line_iter(struct gpiod_line_iter **iter);
 
 bool test_regex_match(const char *str, const char *pattern);
 
index 4555fa44a837afcacefdb95ca391f2f27fb17835..3b7039c8537f7ea0fe1aa684c12a28e6254d8a70 100644 (file)
@@ -116,18 +116,18 @@ TEST_DEFINE(chip_iter_break,
 
 static void line_iter(void)
 {
+       TEST_CLEANUP(test_free_line_iter) struct gpiod_line_iter *iter = NULL;
        TEST_CLEANUP(test_close_chip) struct gpiod_chip *chip = NULL;
-       struct gpiod_line_iter iter;
        struct gpiod_line *line;
        unsigned int i = 0;
 
        chip = gpiod_chip_open(test_chip_path(0));
        TEST_ASSERT_NOT_NULL(chip);
 
-       gpiod_line_iter_init(&iter, chip);
+       iter = gpiod_line_iter_new(chip);
+       TEST_ASSERT_NOT_NULL(iter);
 
-       gpiod_foreach_line(&iter, line) {
-               TEST_ASSERT(!gpiod_line_iter_err(&iter));
+       gpiod_foreach_line(iter, line) {
                TEST_ASSERT_EQ(i, gpiod_line_offset(line));
                i++;
        }
@@ -137,28 +137,3 @@ static void line_iter(void)
 TEST_DEFINE(line_iter,
            "gpiod_line_iter - simple loop, check offsets",
            0, { 8 });
-
-static void line_iter_static_initializer(void)
-{
-       TEST_CLEANUP(test_close_chip) struct gpiod_chip *chip = NULL;
-       struct gpiod_line *line;
-       unsigned int i = 0;
-
-       chip = gpiod_chip_open(test_chip_path(0));
-       TEST_ASSERT_NOT_NULL(chip);
-
-       {
-               struct gpiod_line_iter iter = GPIOD_LINE_ITER_INITIALIZER(chip);
-
-               gpiod_foreach_line(&iter, line) {
-                       TEST_ASSERT(!gpiod_line_iter_err(&iter));
-                       TEST_ASSERT_EQ(i, gpiod_line_offset(line));
-                       i++;
-               }
-       }
-
-       TEST_ASSERT_EQ(8, i);
-}
-TEST_DEFINE(line_iter_static_initializer,
-           "gpiod_line_iter - simple loop, static initializer",
-           0, { 8 });