From: Bartosz Golaszewski Date: Sat, 4 Nov 2017 21:39:36 +0000 (+0100) Subject: iter: rework chip iterators X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=230868dcfadd24c8a1cbad7ee4a42127cd4fe127;p=qemu-gpiodev%2Flibgpiod.git 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 --- diff --git a/include/gpiod.h b/include/gpiod.h index 03c5b23..3276036 100644 --- 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 fc85b58..085f44e 100644 --- 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 1362932..c6fdbaf 100644 --- a/src/lib/iter.c +++ b/src/lib/iter.c @@ -15,21 +15,10 @@ #include #include -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 94163c6..3ba5f7f 100644 --- 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 0dda38c..9bf940c 100644 --- 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 106fc80..4555fa4 100644 --- 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))