From: Michal Bloch <m.bloch@samsung.com>
Date: Thu, 2 Sep 2021 19:01:31 +0000 (+0200)
Subject: Fix a pointless check / race condition
X-Git-Tag: submit/tizen/20210910.102518^0
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=149e1c9b93d7b6e5bbf91edd7d2d3ef30661251d;p=platform%2Fcore%2Fapi%2Fperipheral-io.git

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
--- 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,