path-util: also check absolute path is a directory or not in find_executable()

diff --git a/src/basic/path-util.c b/src/basic/path-util.c
index dae8b7341a5c568d86c76e5b709d6831c95d5002..1644c15fb3bec3bb239483db73661fd39b9d97e0 100644 (file)
--- a/src/basic/path-util.c
+++ b/src/basic/path-util.c
@@ -586,10 +586,11 @@ char* path_join_internal(const char *first, ...) {
 }
 
 static int check_x_access(const char *path, int *ret_fd) {
-        if (ret_fd) {
-                _cleanup_close_ int fd = -1;
-                int r;
+        _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);
@@ -599,14 +600,24 @@ static int check_x_access(const char *path, int *ret_fd) {
                 r = access_fd(fd, X_OK);
                 if (r < 0)
                         return r;
-
-                *ret_fd = TAKE_FD(fd);
         } 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, "/");
+
+        /* If this passes, it must be a directory. */
+        if (access(with_dash, X_OK) >= 0)
+                return -EISDIR;
+
+        if (errno != ENOTDIR)
+                return -errno;
+
+        if (ret_fd)
+                *ret_fd = TAKE_FD(fd);
+
         return 0;
 }
 
@@ -663,32 +674,20 @@ int find_executable_full(const char *name, bool use_path_envvar, char **ret_file
                         return -ENOMEM;
 
                 r = check_x_access(j, ret_fd ? &fd : NULL);
-                if (r >= 0) {
-                        _cleanup_free_ char *with_dash;
-
-                        with_dash = strjoin(j, "/");
-                        if (!with_dash)
-                                return -ENOMEM;
-
-                        /* If this passes, it must be a directory, and so should be skipped. */
-                        if (access(with_dash, X_OK) >= 0)
-                                continue;
-
-                        /* We can't just `continue` inverting this case, since we need to update last_error. */
-                        if (errno == ENOTDIR) {
-                                /* Found it! */
-                                if (ret_filename)
-                                        *ret_filename = path_simplify(TAKE_PTR(j), false);
-                                if (ret_fd)
-                                        *ret_fd = TAKE_FD(fd);
-
-                                return 0;
-                        }
+                if (r < 0) {
+                        /* PATH entries which we don't have access to are ignored, as per tradition. */
+                        if (r != -EACCES)
+                                last_error = r;
+                        continue;
                 }
 
-                /* PATH entries which we don't have access to are ignored, as per tradition. */
-                if (errno != EACCES)
-                        last_error = -errno;
+                /* Found it! */
+                if (ret_filename)
+                        *ret_filename = path_simplify(TAKE_PTR(j), false);
+                if (ret_fd)
+                        *ret_fd = TAKE_FD(fd);
+
+                return 0;
         }
 
         return last_error;