From dfe98084e2641d49eed7d8d5203089031a819632 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Sun, 5 Nov 2017 20:57:25 +0100 Subject: [PATCH] iter: rework line iterator API 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 --- include/gpiod.h | 93 ++++++-------------------------------------- src/lib/helpers.c | 15 ++++--- src/lib/iter.c | 49 ++++++++++++++++++----- src/tools/gpioinfo.c | 15 +++---- tests/gpiod-test.c | 6 +++ tests/gpiod-test.h | 1 + tests/tests-iter.c | 33 ++-------------- 7 files changed, 80 insertions(+), 132 deletions(-) diff --git a/include/gpiod.h b/include/gpiod.h index 8c0ec91..0039cda 100644 --- a/include/gpiod.h +++ b/include/gpiod.h @@ -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)) /** diff --git a/src/lib/helpers.c b/src/lib/helpers.c index 085f44e..3b0bcf9 100644 --- a/src/lib/helpers.c +++ b/src/lib/helpers.c @@ -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; } diff --git a/src/lib/iter.c b/src/lib/iter.c index c6fdbaf..62a4165 100644 --- a/src/lib/iter.c +++ b/src/lib/iter.c @@ -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; } diff --git a/src/tools/gpioinfo.c b/src/tools/gpioinfo.c index 9bf940c..4a13ec1 100644 --- a/src/tools/gpioinfo.c +++ b/src/tools/gpioinfo.c @@ -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) diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c index d64a24d..0c68e8c 100644 --- a/tests/gpiod-test.c +++ b/tests/gpiod-test.c @@ -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) diff --git a/tests/gpiod-test.h b/tests/gpiod-test.h index 0f45188..44cabff 100644 --- a/tests/gpiod-test.h +++ b/tests/gpiod-test.h @@ -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); diff --git a/tests/tests-iter.c b/tests/tests-iter.c index 4555fa4..3b7039c 100644 --- a/tests/tests-iter.c +++ b/tests/tests-iter.c @@ -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 }); -- 2.30.2