]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
shared/install: make install_changes_add propagate passed-in errno value
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Thu, 20 Oct 2022 09:41:53 +0000 (11:41 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Mon, 24 Oct 2022 10:37:19 +0000 (12:37 +0200)
The function was written to only return an error from internal allocation
failures, because when using it to create a bus message, we want to distinguish
a failed operation from an allocation error when sending the reply. But it
turns out that the only caller that makes this distinction checks that the
passed-in errno value ('type') is not negative beforehand. So we can make the
function pass 'type' value through, which makes most of the callers nicer.

No functional change.

src/shared/install.c
src/shared/install.h

index 4cc0b67b8b477f826462307ec35e098b62cf0c15..01d4c2ac9644dc5ac86dec60bc3214b31b52a2ba 100644 (file)
@@ -265,7 +265,7 @@ static const char* config_path_from_flags(const LookupPaths *lp, UnitFileFlags f
                 return FLAGS_SET(flags, UNIT_FILE_RUNTIME) ? lp->runtime_config : lp->persistent_config;
 }
 
-int install_changes_add(
+InstallChangeType install_changes_add(
                 InstallChange **changes,
                 size_t *n_changes,
                 InstallChangeType type, /* INSTALL_CHANGE_SYMLINK, _UNLINK, _IS_MASKED, _IS_DANGLING, … if positive or errno if negative */
@@ -278,8 +278,11 @@ int install_changes_add(
         assert(!changes == !n_changes);
         assert(INSTALL_CHANGE_TYPE_VALID(type));
 
+        /* Register a change or error. Note that the return value may be the error
+         * that was passed in, or -ENOMEM generated internally. */
+
         if (!changes)
-                return 0;
+                return type;
 
         c = reallocarray(*changes, *n_changes + 1, sizeof(InstallChange));
         if (!c)
@@ -308,7 +311,7 @@ int install_changes_add(
                 .source = TAKE_PTR(s),
         };
 
-        return 0;
+        return type;
 }
 
 void install_changes_free(InstallChange *changes, size_t n_changes) {
@@ -515,10 +518,8 @@ static int create_symlink(
                 return 1;
         }
 
-        if (errno != EEXIST) {
-                install_changes_add(changes, n_changes, -errno, new_path, NULL);
-                return -errno;
-        }
+        if (errno != EEXIST)
+                return install_changes_add(changes, n_changes, -errno, new_path, NULL);
 
         r = readlink_malloc(new_path, &dest);
         if (r < 0) {
@@ -526,8 +527,7 @@ static int create_symlink(
                 if (r == -EINVAL)
                         r = -EEXIST;
 
-                install_changes_add(changes, n_changes, r, new_path, NULL);
-                return r;
+                return install_changes_add(changes, n_changes, r, new_path, NULL);
         }
 
         if (chroot_unit_symlinks_equivalent(lp, new_path, dest, old_path)) {
@@ -536,16 +536,12 @@ static int create_symlink(
                 return 1;
         }
 
-        if (!force) {
-                install_changes_add(changes, n_changes, -EEXIST, new_path, dest);
-                return -EEXIST;
-        }
+        if (!force)
+                return install_changes_add(changes, n_changes, -EEXIST, new_path, dest);
 
         r = symlink_atomic(old_path, new_path);
-        if (r < 0) {
-                install_changes_add(changes, n_changes, r, new_path, NULL);
-                return r;
-        }
+        if (r < 0)
+                return install_changes_add(changes, n_changes, r, new_path, NULL);
 
         install_changes_add(changes, n_changes, INSTALL_CHANGE_UNLINK, new_path, NULL);
         install_changes_add(changes, n_changes, INSTALL_CHANGE_SYMLINK, new_path, old_path);
@@ -1093,15 +1089,11 @@ static int install_info_may_process(
         /* Checks whether the loaded unit file is one we should process, or is masked,
          * transient or generated and thus not subject to enable/disable operations. */
 
-        if (i->install_mode == INSTALL_MODE_MASKED) {
-                install_changes_add(changes, n_changes, -ERFKILL, i->path, NULL);
-                return -ERFKILL;
-        }
+        if (i->install_mode == INSTALL_MODE_MASKED)
+                return install_changes_add(changes, n_changes, -ERFKILL, i->path, NULL);
         if (path_is_generator(lp, i->path) ||
-            path_is_transient(lp, i->path)) {
-                install_changes_add(changes, n_changes, -EADDRNOTAVAIL, i->path, NULL);
-                return -EADDRNOTAVAIL;
-        }
+            path_is_transient(lp, i->path))
+                return install_changes_add(changes, n_changes, -EADDRNOTAVAIL, i->path, NULL);
 
         return 0;
 }
@@ -1854,13 +1846,11 @@ int unit_file_verify_alias(
                 r = unit_validate_alias_symlink_or_warn(LOG_DEBUG, dst_updated ?: dst, info->name);
                 if (r == -ELOOP)  /* -ELOOP means self-alias, which we (quietly) ignore */
                         return r;
-                if (r < 0) {
-                        install_changes_add(changes, n_changes,
-                                              r == -EINVAL ? -EXDEV : r,
-                                              dst_updated ?: dst,
-                                              info->name);
-                        return r;
-                }
+                if (r < 0)
+                        return install_changes_add(changes, n_changes,
+                                                   r == -EINVAL ? -EXDEV : r,
+                                                   dst_updated ?: dst,
+                                                   info->name);
         }
 
         *ret_dst = TAKE_PTR(dst_updated);
@@ -1952,10 +1942,8 @@ static int install_info_symlink_wants(
                 if (r < 0)
                         return r;
 
-                if (instance.install_mode == INSTALL_MODE_MASKED) {
-                        install_changes_add(changes, n_changes, -ERFKILL, instance.path, NULL);
-                        return -ERFKILL;
-                }
+                if (instance.install_mode == INSTALL_MODE_MASKED)
+                        return install_changes_add(changes, n_changes, -ERFKILL, instance.path, NULL);
 
                 n = instance.name;
 
@@ -1971,10 +1959,8 @@ static int install_info_symlink_wants(
                 _cleanup_free_ char *path = NULL, *dst = NULL;
 
                 q = install_name_printf(scope, info, *s, &dst);
-                if (q < 0) {
-                        install_changes_add(changes, n_changes, q, *s, NULL);
-                        return q;
-                }
+                if (q < 0)
+                        return install_changes_add(changes, n_changes, q, *s, NULL);
 
                 if (!unit_name_is_valid(dst, valid_dst_type)) {
                         /* Generate a proper error here: EUCLEAN if the name is generally bad, EIDRM if the
@@ -1987,13 +1973,10 @@ static int install_info_symlink_wants(
                         if (file_flags & UNIT_FILE_IGNORE_AUXILIARY_FAILURE)
                                 continue;
 
-                        if (unit_name_is_valid(dst, UNIT_NAME_ANY)) {
-                                install_changes_add(changes, n_changes, -EIDRM, dst, n);
-                                r = -EIDRM;
-                        } else {
-                                install_changes_add(changes, n_changes, -EUCLEAN, dst, NULL);
-                                r = -EUCLEAN;
-                        }
+                        if (unit_name_is_valid(dst, UNIT_NAME_ANY))
+                                return install_changes_add(changes, n_changes, -EIDRM, dst, n);
+                        else
+                                return install_changes_add(changes, n_changes, -EUCLEAN, dst, NULL);
 
                         continue;
                 }
@@ -2122,8 +2105,7 @@ static int install_context_apply(
                                 continue;
                         }
 
-                        install_changes_add(changes, n_changes, q, i->name, NULL);
-                        return q;
+                        return install_changes_add(changes, n_changes, q, i->name, NULL);
                 }
 
                 /* We can attempt to process a masked unit when a different unit
index d13b143e4eb3abd0426444a7ba3727c393a45744..9bb412ba06842031377d903d6bde981c92416f2f 100644 (file)
@@ -197,7 +197,7 @@ int unit_file_exists(LookupScope scope, const LookupPaths *paths, const char *na
 int unit_file_get_list(LookupScope scope, const char *root_dir, Hashmap *h, char **states, char **patterns);
 Hashmap* unit_file_list_free(Hashmap *h);
 
-int install_changes_add(InstallChange **changes, size_t *n_changes, int type, const char *path, const char *source);
+InstallChangeType install_changes_add(InstallChange **changes, size_t *n_changes, int type, const char *path, const char *source);
 void install_changes_free(InstallChange *changes, size_t n_changes);
 void install_changes_dump(int r, const char *verb, const InstallChange *changes, size_t n_changes, bool quiet);