From f4e3b51f92c2e13c2c5a29dd9b063553d8f7a373 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Fri, 17 Nov 2017 15:03:14 +0100 Subject: [PATCH] core: improve error handling Rework the way we verify that line bulk objects are consistent: split the verify_line_bulk() function into sub-routines, make the names more self-explanatory and add other minor fixes & tweaks. Improve the consistency of errno codes: set errno to EPERM if the line for which gpiod_line_event_get_fd() is called is not configured for event monitoring. We were already testing that line requests fail if we pass them a line bulk containing lines from different chips, but add an analogous test for gpiod_line_get_value_bulk(). Signed-off-by: Bartosz Golaszewski --- src/lib/core.c | 68 ++++++++++++++++++++++++++++++--------------- tests/tests-event.c | 2 +- tests/tests-line.c | 55 ++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 24 deletions(-) diff --git a/src/lib/core.c b/src/lib/core.c index d285d9c..90666c5 100644 --- a/src/lib/core.c +++ b/src/lib/core.c @@ -274,32 +274,50 @@ int gpiod_line_update(struct gpiod_line *line) return 0; } -static bool line_bulk_is_requested(struct gpiod_line_bulk *bulk) +static bool line_bulk_same_chip(struct gpiod_line_bulk *bulk) { - struct gpiod_line *line, **lineptr; + struct gpiod_line *first_line, *line; + struct gpiod_chip *first_chip, *chip; + unsigned int i; - gpiod_line_bulk_foreach_line(bulk, line, lineptr) { - if (!gpiod_line_is_requested(line)) + if (bulk->num_lines == 1) + return true; + + first_line = gpiod_line_bulk_get_line(bulk, 0); + first_chip = gpiod_line_get_chip(first_line); + + for (i = 1; i < bulk->num_lines; i++) { + line = bulk->lines[i]; + chip = gpiod_line_get_chip(line); + + if (first_chip != chip) { + errno = EINVAL; return false; + } } return true; } -static bool verify_line_bulk(struct gpiod_line_bulk *bulk) +static bool line_bulk_all_requested(struct gpiod_line_bulk *bulk) { - struct gpiod_line *line; - struct gpiod_chip *chip; - unsigned int off; - - chip = gpiod_line_get_chip(gpiod_line_bulk_get_line(bulk, 0)); + struct gpiod_line *line, **lineptr; - gpiod_line_bulk_foreach_line_off(bulk, line, off) { - if (off > 0 && chip != gpiod_line_get_chip(line)) { - errno = EINVAL; + gpiod_line_bulk_foreach_line(bulk, line, lineptr) { + if (!gpiod_line_is_requested(line)) { + errno = EPERM; return false; } + } + return true; +} + +static bool line_bulk_all_free(struct gpiod_line_bulk *bulk) +{ + struct gpiod_line *line, **lineptr; + + gpiod_line_bulk_foreach_line(bulk, line, lineptr) { if (!gpiod_line_is_free(line)) { errno = EBUSY; return false; @@ -460,7 +478,7 @@ int gpiod_line_request_bulk(struct gpiod_line_bulk *bulk, const struct gpiod_line_request_config *config, const int *default_vals) { - if (!verify_line_bulk(bulk)) + if (!line_bulk_same_chip(bulk) || !line_bulk_all_free(bulk)) return -1; if (line_request_is_direction(config->request_type)) { @@ -528,12 +546,10 @@ int gpiod_line_get_value_bulk(struct gpiod_line_bulk *bulk, int *values) unsigned int i; int status, fd; - first = gpiod_line_bulk_get_line(bulk, 0); - - if (!line_bulk_is_requested(bulk)) { - errno = EPERM; + if (!line_bulk_same_chip(bulk) || !line_bulk_all_requested(bulk)) return -1; - } + + first = gpiod_line_bulk_get_line(bulk, 0); memset(&data, 0, sizeof(data)); @@ -566,10 +582,8 @@ int gpiod_line_set_value_bulk(struct gpiod_line_bulk *bulk, int *values) unsigned int i; int status; - if (!line_bulk_is_requested(bulk)) { - errno = EPERM; + if (!line_bulk_same_chip(bulk) || !line_bulk_all_requested(bulk)) return -1; - } memset(&data, 0, sizeof(data)); @@ -604,6 +618,9 @@ int gpiod_line_event_wait_bulk(struct gpiod_line_bulk *bulk, struct gpiod_line *line; int rv; + if (!line_bulk_same_chip(bulk) || !line_bulk_all_requested(bulk)) + return -1; + memset(fds, 0, sizeof(fds)); num_lines = gpiod_line_bulk_num_lines(bulk); @@ -637,13 +654,18 @@ int gpiod_line_event_wait_bulk(struct gpiod_line_bulk *bulk, int gpiod_line_event_read(struct gpiod_line *line, struct gpiod_line_event *event) { + if (line->state != LINE_REQUESTED_EVENTS) { + errno = EPERM; + return -1; + } + return gpiod_line_event_read_fd(line->fd, event); } int gpiod_line_event_get_fd(struct gpiod_line *line) { if (line->state != LINE_REQUESTED_EVENTS) { - errno = EINVAL; + errno = EPERM; return -1; } diff --git a/tests/tests-event.c b/tests/tests-event.c index a5bb28b..95a3d01 100644 --- a/tests/tests-event.c +++ b/tests/tests-event.c @@ -223,7 +223,7 @@ static void event_get_fd_when_values_requested(void) fd = gpiod_line_event_get_fd(line); TEST_ASSERT_EQ(fd, -1); - TEST_ASSERT_ERRNO_IS(EINVAL); + TEST_ASSERT_ERRNO_IS(EPERM); } TEST_DEFINE(event_get_fd_when_values_requested, "events - gpiod_line_event_get_fd(): line requested for values", diff --git a/tests/tests-line.c b/tests/tests-line.c index 220255d..ac258dd 100644 --- a/tests/tests-line.c +++ b/tests/tests-line.c @@ -278,6 +278,61 @@ TEST_DEFINE(line_set_value, "gpiod_line_set_value() - good", 0, { 8 }); +static void line_get_value_different_chips(void) +{ + TEST_CLEANUP(test_close_chip) struct gpiod_chip *chipA = NULL; + TEST_CLEANUP(test_close_chip) struct gpiod_chip *chipB = NULL; + struct gpiod_line *lineA1, *lineA2, *lineB1, *lineB2; + struct gpiod_line_bulk bulkA, bulkB, bulk; + int rv, vals[4]; + + chipA = gpiod_chip_open(test_chip_path(0)); + TEST_ASSERT_NOT_NULL(chipA); + + chipB = gpiod_chip_open(test_chip_path(1)); + TEST_ASSERT_NOT_NULL(chipB); + + lineA1 = gpiod_chip_get_line(chipA, 3); + lineA2 = gpiod_chip_get_line(chipA, 4); + lineB1 = gpiod_chip_get_line(chipB, 5); + lineB2 = gpiod_chip_get_line(chipB, 6); + TEST_ASSERT_NOT_NULL(lineA1); + TEST_ASSERT_NOT_NULL(lineA2); + TEST_ASSERT_NOT_NULL(lineB1); + TEST_ASSERT_NOT_NULL(lineB2); + + gpiod_line_bulk_init(&bulkA); + gpiod_line_bulk_init(&bulkB); + gpiod_line_bulk_init(&bulk); + + gpiod_line_bulk_add(&bulk, lineA1); + gpiod_line_bulk_add(&bulk, lineA2); + gpiod_line_bulk_add(&bulk, lineB1); + gpiod_line_bulk_add(&bulk, lineB2); + + gpiod_line_bulk_add(&bulkA, lineA1); + gpiod_line_bulk_add(&bulkA, lineA2); + gpiod_line_bulk_add(&bulkB, lineB1); + gpiod_line_bulk_add(&bulkB, lineB2); + gpiod_line_bulk_add(&bulk, lineA1); + gpiod_line_bulk_add(&bulk, lineA2); + gpiod_line_bulk_add(&bulk, lineB1); + gpiod_line_bulk_add(&bulk, lineB2); + + rv = gpiod_line_request_bulk_input(&bulkA, TEST_CONSUMER); + TEST_ASSERT_RET_OK(rv); + + rv = gpiod_line_request_bulk_input(&bulkB, TEST_CONSUMER); + TEST_ASSERT_RET_OK(rv); + + rv = gpiod_line_get_value_bulk(&bulk, vals); + TEST_ASSERT_EQ(rv, -1); + TEST_ASSERT_ERRNO_IS(EINVAL); +} +TEST_DEFINE(line_get_value_different_chips, + "gpiod_line_get_value_bulk() - different chips", + 0, { 8, 8 }); + static void line_get_good(void) { TEST_CLEANUP(test_line_close_chip) struct gpiod_line *line = NULL; -- 2.30.2