Thread (3 messages) flat view 3 messages, 3 authors, 2021-09-22

Re: [PATCH libgpiod-v2] tools: Restore support for opening chips by label

From: Bartosz Golaszewski <hidden>
Date: 2021-09-20 14:33:32

On Wed, Jul 28, 2021 at 11:19 PM Ben Hutchings [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Support for opening chips by label was removed because labels
are not necessarily unique and lookup by label requires opening
every GPIO device.

I don't think these concerns apply to the tools.  They will normally
be run by root, and if a label is specified it's because it's known to
be unique.

This adds a chip_open_by_label() function to tools-common.c, which:

1. Scans the /dev/ directory for GPIO devices, and opens each in turn
2. Checks whether the label matches, and that the label isn't used by
   two distinct devices
3. If all devices can be opened and exactly one matches the label,
   return that device

It changes chip_open_lookup() to call chip_open_by_label() as a final
fallback, rather than the previous behaviour.  This should avoid
producing spurious error messages if a tool is run by a user that can
only access a subset of GPIO devices.

Signed-off-by: Ben Hutchings <redacted>
---
 tools/tools-common.c | 63 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)
diff --git a/tools/tools-common.c b/tools/tools-common.c
index 36724d5..d2665e8 100644
--- a/tools/tools-common.c
+++ b/tools/tools-common.c
@@ -4,6 +4,7 @@
 /* Common code for GPIO tools. */

 #include <ctype.h>
+#include <dirent.h>
 #include <errno.h>
 #include <gpiod.h>
 #include <libgen.h>
@@ -146,6 +147,66 @@ static struct gpiod_chip *chip_open_by_number(unsigned int num)
        return chip;
 }

+struct gpiod_chip *chip_open_by_label(const char *label)
+{
+       struct gpiod_chip *chip = NULL, *next = NULL;
+       struct dirent **entries;
+       int num_chips, i, error = 0;
+
+       num_chips = scandir("/dev/", &entries, chip_dir_filter, alphasort);
+       if (num_chips < 0) {
+               error = errno;
+               fprintf(stderr, "unable to scan /dev: %s\n", strerror(error));
+               goto out;
+       }
+
+       for (i = 0; i < num_chips; i++) {
+               next = chip_open_by_name(entries[i]->d_name);
+               if (!next) {
+                       error = errno;
+                       fprintf(stderr, "unable to open %s: %s\n",
+                               entries[i]->d_name, strerror(error));
How about using access() to check the permissions first? This way we
wouldn't need to spam the user with error messages - we'd just
silently ignore chips we don't have access to.

Bart
quoted hunk ↗ jump to hunk
+                       break;
+               }
+
+               if (chip && !strcmp(gpiod_chip_get_name(chip),
+                                   gpiod_chip_get_name(next))) {
+                       /* chip and next are actually the same device */
+                       gpiod_chip_close(next);
+               } else if (!strcmp(gpiod_chip_get_label(next), label)) {
+                       /* label matches; check it's not a duplicate */
+                       if (chip) {
+                               fprintf(stderr,
+                                       "multiple chips have the label \"%s\"\n",
+                                       label);
+                               error = EINVAL;
+                               break;
+                       }
+                       chip = next;
+               } else {
+                       gpiod_chip_close(next);
+               }
+       }
+
+       if (error) {
+               if (chip)
+                       gpiod_chip_close(chip);
+               if (next)
+                       gpiod_chip_close(next);
+               chip = NULL;
+       } else if (!chip) {
+               error = ENOENT;
+       }
+
+       for (i = 0; i < num_chips; i++)
+               free(entries[i]);
+       free(entries);
+
+out:
+       errno = error;
+       return chip;
+}
+
 static bool isuint(const char *str)
 {
        for (; *str && isdigit(*str); str++)
@@ -166,6 +227,8 @@ struct gpiod_chip *chip_open_lookup(const char *device)
                else
                        chip = gpiod_chip_open(device);
        }
+       if (!chip)
+               chip = chip_open_by_label(device);

        return chip;
 }
--
2.20.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help