On Mon, Sep 20, 2021 at 04:32:22PM +0200, Bartosz Golaszewski wrote:

On Wed, Jul 28, 2021 at 11:19 PM Ben Hutchings [off-list ref] wrote:

quoted

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.

[...]

quoted

+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.

[...]

I don't see any benefit in using access() rather than checking for
EACCES; that just seems to add a race condition.

As for whether the error should be reported, this is consistent with
the old behaviour and I did not want to report that "chip label does
not exist" in case of missing permission.  And again, I would expect
the tools to be run as root, so this shouldn't happen in practice.

Perhaps a better approach would be to record any access failure and
then report it at the end only if the label was not found?

Ben.

-- 
Ben Hutchings · Senior Embedded Software Engineer, Essensium-Mind · mind.be