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 <bartekgola@gmail.com>

diff --git a/src/lib/core.c b/src/lib/core.c
index d285d9cf6827b2c9b5436e0da7e2abc4ec086a50..90666c513156010a5ed169f00ccbac44424a4c88 100644 (file)
--- 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 a5bb28b2387e2521e689b14c9055319c28b1b00b..95a3d012de2806a1586bcbf575af5c089f1fd15e 100644 (file)
--- 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 220255df829cd5720fb2269575ae83c636c2d04a..ac258dde84681d353b9452fd39c320d75f9720a0 100644 (file)
--- 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;