Fix a pointless check / race condition

Using the open() call already tells us whether the file exists,
so there is little point checking that separately. Additionally
it's a minor race condition (the file can stop existing between
the calls, though no harm done in this particular case).

Add some commentary as well because the chunk is a bit confusing.

Change-Id: I8c3469aaddac2b30d9f60c0ce65abcb3bd9847b9
Signed-off-by: Michal Bloch <m.bloch@samsung.com>

diff --git a/src/peripheral_uart.c b/src/peripheral_uart.c
index 29bbc0b..8453095 100644 (file)
--- a/src/peripheral_uart.c
+++ b/src/peripheral_uart.c
@@ -119,24 +119,38 @@ int peripheral_uart_open_flags(int port, peripheral_open_flags_e flags, peripher
                "/dev/ttySAC",
        };
 
-       size_t index = 0;
-       int retval;
        char path[DEV_PATH_FMT_MAX_SIZE] = {0, }; /* space for /dev/ttyXXX%d */
-
-       retval = peripheral_uart_find_devpath(port, path, DEV_PATH_FMT_MAX_SIZE);
-       if (retval < 0 || access(path, F_OK) != 0) {
-               for (index = 0; index < ARRAY_SIZE(sysfs_uart_path); index++) {
+       const int FLAGS = O_RDWR | O_NOCTTY | O_NONBLOCK | O_CLOEXEC;
+
+       int fd = -1;
+       int retval = peripheral_uart_find_devpath(port, path, DEV_PATH_FMT_MAX_SIZE);
+       if (retval >= 0)
+               fd = open(path, FLAGS);
+       else
+               errno = ENOENT;
+
+       /* We try the other paths iteratively either if find_devpath() fails,
+        * or if nothing exists under the path "successfully" found.
+        *
+        * In particular, if something exists but we fail to open it,
+        * for example due to access control, we don't try further and
+        * the function fails immediately.
+        *
+        * This is so that various clients will only try to access
+        * the same device consistently, regardless of app rights. */
+       if (fd == -1 && errno == ENOENT) {
+               errno = 0;
+               for (size_t index = 0; index < ARRAY_SIZE(sysfs_uart_path); index++) {
                        snprintf(path, sizeof path, DEV_PATH_FMT, sysfs_uart_path[index], port);
-                       if (access(path, F_OK) == 0)
+                       fd = open(path, FLAGS);
+                       if (fd != -1 || errno != ENOENT)
                                break;
                }
+               if (fd == -1 && errno == ENOENT)
+                       return PERIPHERAL_ERROR_NO_DEVICE;
        }
-
-       if (index == ARRAY_SIZE(sysfs_uart_path))
-               return PERIPHERAL_ERROR_NO_DEVICE;
-
-       handle->fd = open(path, O_RDWR | O_NOCTTY | O_NONBLOCK | O_CLOEXEC);
-       CHECK_ERROR(handle->fd < 0);
+       CHECK_ERROR(fd < 0);
+       handle->fd = fd;
 
        int ret = PERIPHERAL_ERROR_NONE;
        TRY_FLOCK(ret, handle->fd, peripheral_get_lock_type(flags) | LOCK_NB,