basic/fs-util: change CHASE_OPEN flag into a separate output parameter

chase_symlinks() would return negative on error, and either a non-negative status
or a non-negative fd when CHASE_OPEN was given. This made the interface quite
complicated, because dependning on the flags used, we would get two different
"types" of return object. Coverity was always confused by this, and flagged
every use of chase_symlinks() without CHASE_OPEN as a resource leak (because it
would this that an fd is returned). This patch uses a saparate output parameter,
so there is no confusion.

(I think it is OK to have functions which return either an error or an fd. It's
only returning *either* an fd or a non-fd that is confusing.)

6 files changed, 19 insertions, 17 deletions

diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c
index a54714169e..3e75464966 100644
--- a/src/shared/dissect-image.c
+++ b/src/shared/dissect-image.c
@@ -833,7 +833,7 @@ static int mount_partition(
         rw = m->rw && !(flags & DISSECT_IMAGE_READ_ONLY);
 
         if (directory) {
-                r = chase_symlinks(directory, where, CHASE_PREFIX_ROOT, &chased);
+                r = chase_symlinks(directory, where, CHASE_PREFIX_ROOT, &chased, NULL);
                 if (r < 0)
                         return r;
 
@@ -909,7 +909,7 @@ int dissected_image_mount(DissectedImage *m, const char *where, uid_t uid_shift,
                 /* Mount the ESP to /efi if it exists. If it doesn't exist, use /boot instead, but only if it
                  * exists and is empty, and we didn't already mount the XBOOTLDR partition into it. */
 
-                r = chase_symlinks("/efi", where, CHASE_PREFIX_ROOT, NULL);
+                r = chase_symlinks("/efi", where, CHASE_PREFIX_ROOT, NULL, NULL);
                 if (r >= 0) {
                         r = mount_partition(m->partitions + PARTITION_ESP, where, "/efi", uid_shift, flags);
                         if (r < 0)
@@ -918,7 +918,7 @@ int dissected_image_mount(DissectedImage *m, const char *where, uid_t uid_shift,
                 } else if (boot_mounted <= 0) {
                         _cleanup_free_ char *p = NULL;
 
-                        r = chase_symlinks("/boot", where, CHASE_PREFIX_ROOT, &p);
+                        r = chase_symlinks("/boot", where, CHASE_PREFIX_ROOT, &p, NULL);
                         if (r >= 0 && dir_is_empty(p) > 0) {
                                 r = mount_partition(m->partitions + PARTITION_ESP, where, "/boot", uid_shift, flags);
                                 if (r < 0)
diff --git a/src/shared/dropin.c b/src/shared/dropin.c
index 2f67a44bf0..7328b7adde 100644
--- a/src/shared/dropin.c
+++ b/src/shared/dropin.c
@@ -106,7 +106,7 @@ static int unit_file_add_dir(
 
         /* This adds [original_root]/path to dirs, if it exists. */
 
-        r = chase_symlinks(path, original_root, 0, &chased);
+        r = chase_symlinks(path, original_root, 0, &chased, NULL);
         if (r == -ENOENT) /* Ignore -ENOENT, after all most units won't have a drop-in dir. */
                 return 0;
         if (r == -ENAMETOOLONG) {
diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index 7007374192..cceed58f74 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -1113,7 +1113,7 @@ int image_read_metadata(Image *i) {
                 _cleanup_free_ char *hostname = NULL;
                 _cleanup_free_ char *path = NULL;
 
-                r = chase_symlinks("/etc/hostname", i->path, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &path);
+                r = chase_symlinks("/etc/hostname", i->path, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &path, NULL);
                 if (r < 0 && r != -ENOENT)
                         log_debug_errno(r, "Failed to chase /etc/hostname in image %s: %m", i->name);
                 else if (r >= 0) {
@@ -1124,7 +1124,7 @@ int image_read_metadata(Image *i) {
 
                 path = mfree(path);
 
-                r = chase_symlinks("/etc/machine-id", i->path, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &path);
+                r = chase_symlinks("/etc/machine-id", i->path, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &path, NULL);
                 if (r < 0 && r != -ENOENT)
                         log_debug_errno(r, "Failed to chase /etc/machine-id in image %s: %m", i->name);
                 else if (r >= 0) {
@@ -1142,7 +1142,7 @@ int image_read_metadata(Image *i) {
 
                 path = mfree(path);
 
-                r = chase_symlinks("/etc/machine-info", i->path, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &path);
+                r = chase_symlinks("/etc/machine-info", i->path, CHASE_PREFIX_ROOT|CHASE_TRAIL_SLASH, &path, NULL);
                 if (r < 0 && r != -ENOENT)
                         log_debug_errno(r, "Failed to chase /etc/machine-info in image %s: %m", i->name);
                 else if (r >= 0) {
diff --git a/src/shared/os-util.c b/src/shared/os-util.c
index 2191a610ae..b2af8535f9 100644
--- a/src/shared/os-util.c
+++ b/src/shared/os-util.c
@@ -33,22 +33,24 @@ int path_is_os_tree(const char *path) {
 int open_os_release(const char *root, char **ret_path, int *ret_fd) {
         _cleanup_free_ char *q = NULL;
         const char *p;
-        int k;
+        int r, fd;
 
         FOREACH_STRING(p, "/etc/os-release", "/usr/lib/os-release") {
-                k = chase_symlinks(p, root, CHASE_PREFIX_ROOT|(ret_fd ? CHASE_OPEN : 0), (ret_path ? &q : NULL));
-                if (k != -ENOENT)
+                r = chase_symlinks(p, root, CHASE_PREFIX_ROOT,
+                                   ret_path ? &q : NULL,
+                                   ret_fd ? &fd : NULL);
+                if (r != -ENOENT)
                         break;
         }
-        if (k < 0)
-                return k;
+        if (r < 0)
+                return r;
 
         if (ret_fd) {
                 int real_fd;
 
                 /* Convert the O_PATH fd into a proper, readable one */
-                real_fd = fd_reopen(k, O_RDONLY|O_CLOEXEC|O_NOCTTY);
-                safe_close(k);
+                real_fd = fd_reopen(fd, O_RDONLY|O_CLOEXEC|O_NOCTTY);
+                safe_close(fd);
                 if (real_fd < 0)
                         return real_fd;
 
diff --git a/src/shared/switch-root.c b/src/shared/switch-root.c
index f721aff760..a807826378 100644
--- a/src/shared/switch-root.c
+++ b/src/shared/switch-root.c
@@ -52,7 +52,7 @@ int switch_root(const char *new_root,
         }
 
         /* Determine where we shall place the old root after the transition */
-        r = chase_symlinks(old_root_after, new_root, CHASE_PREFIX_ROOT|CHASE_NONEXISTENT, &resolved_old_root_after);
+        r = chase_symlinks(old_root_after, new_root, CHASE_PREFIX_ROOT|CHASE_NONEXISTENT, &resolved_old_root_after, NULL);
         if (r < 0)
                 return log_error_errno(r, "Failed to resolve %s/%s: %m", new_root, old_root_after);
         if (r == 0) /* Doesn't exist yet. Let's create it */
@@ -68,7 +68,7 @@ int switch_root(const char *new_root,
         FOREACH_STRING(i, "/sys", "/dev", "/run", "/proc") {
                 _cleanup_free_ char *chased = NULL;
 
-                r = chase_symlinks(i, new_root, CHASE_PREFIX_ROOT|CHASE_NONEXISTENT, &chased);
+                r = chase_symlinks(i, new_root, CHASE_PREFIX_ROOT|CHASE_NONEXISTENT, &chased, NULL);
                 if (r < 0)
                         return log_error_errno(r, "Failed to resolve %s/%s: %m", new_root, i);
                 if (r > 0) {
diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c
index b015ff9338..28cd3c8600 100644
--- a/src/shared/unit-file.c
+++ b/src/shared/unit-file.c
@@ -303,7 +303,7 @@ int unit_file_build_name_map(
                                 }
 
                                 /* Get rid of "." and ".." components in target path */
-                                r = chase_symlinks(target, lp->root_dir, CHASE_NOFOLLOW | CHASE_NONEXISTENT, &simplified);
+                                r = chase_symlinks(target, lp->root_dir, CHASE_NOFOLLOW | CHASE_NONEXISTENT, &simplified, NULL);
                                 if (r < 0) {
                                         log_warning_errno(r, "Failed to resolve symlink %s pointing to %s, ignoring: %m",
                                                           filename, target);