path-util: simplify check_x_access()

Follow-up for ece852c84592220c3d6bb5a055fd8b84ea097290.

This addresses the following comments by the Lennart:
---
hmm, so this now does two access() calls for the case where the fd is
not requested, and opens things up for races (theoretically, …). now,
the access() code path was in place for optimization, but if an optimization
is less sexy than the original (and i think it is less sexy, since more
than one syscall, and non-atomic), i think we shouldn't do the optimization.

maybe we should just always use open(O_PATH) now, and then fstat() it to
check if regular file, and then access_fd() it for checking if its executable.

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index 1644c15fb3bec3bb239483db73661fd39b9d97e0..5bcbc7a7947278058c274b6b757cc7c34740aba9 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -587,33 +587,21 @@ char* path_join_internal(const char *first, ...) {
 
 static int check_x_access(const char *path, int *ret_fd) {
         _cleanup_close_ int fd = -1;
-        const char *with_dash;
         int r;
 
-        if (ret_fd) {
-                /* We need to use O_PATH because there may be executables for which we have only exec
-                 * permissions, but not read (usually suid executables). */
-                fd = open(path, O_PATH|O_CLOEXEC);
-                if (fd < 0)
-                        return -errno;
-
-                r = access_fd(fd, X_OK);
-                if (r < 0)
-                        return r;
-        } else {
-                /* Let's optimize things a bit by not opening the file if we don't need the fd. */
-                if (access(path, X_OK) < 0)
-                        return -errno;
-        }
-
-        with_dash = strjoina(path, "/");
+        /* We need to use O_PATH because there may be executables for which we have only exec
+         * permissions, but not read (usually suid executables). */
+        fd = open(path, O_PATH|O_CLOEXEC);
+        if (fd < 0)
+                return -errno;
 
-        /* If this passes, it must be a directory. */
-        if (access(with_dash, X_OK) >= 0)
-                return -EISDIR;
+        r = fd_verify_regular(fd);
+        if (r < 0)
+                return r;
 
-        if (errno != ENOTDIR)
-                return -errno;
+        r = access_fd(fd, X_OK);
+        if (r < 0)
+                return r;
 
         if (ret_fd)
                 *ret_fd = TAKE_FD(fd);