From 422f80d59bf4f864b51282e4de9f4c3bd0513ec6 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 18 Jun 2024 17:09:21 +0200
Subject: [PATCH 1/2] install: collect more install_changes_add() errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We so far collected most unexpected errors from install_changes_add()
and propagated them – but for some invocations we forgot to do that. Add
that, and take care we only propagated unexpected errors (i.e. ENOMEM
and such), but treat expected errors as before.

Follow-up for 5163c9b1e56293b1bb2803420613c5b374570892
---
 src/shared/install.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index ca8e7d2733f..70ba230ac16 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1810,7 +1810,8 @@ static int install_info_discover(
                 r = install_info_traverse(ctx, lp, info, flags, ret);
 
         if (r < 0)
-                install_changes_add(changes, n_changes, r, name_or_path, NULL);
+                return install_changes_add(changes, n_changes, r, name_or_path, NULL);
+
         return r;
 }
 
@@ -1871,7 +1872,10 @@ int unit_file_verify_alias(
                 if (!p)
                         p = endswith(dir, ".requires");
                 if (!p) {
-                        install_changes_add(changes, n_changes, -EXDEV, dst, NULL);
+                        r = install_changes_add(changes, n_changes, -EXDEV, dst, NULL);
+                        if (r != -EXDEV)
+                                return r;
+
                         return log_debug_errno(SYNTHETIC_ERRNO(EXDEV), "Invalid path \"%s\" in alias.", dir);
                 }
 
@@ -1879,7 +1883,9 @@ int unit_file_verify_alias(
 
                 UnitNameFlags type = unit_name_classify(dir);
                 if (type < 0) {
-                        install_changes_add(changes, n_changes, -EXDEV, dst, NULL);
+                        r = install_changes_add(changes, n_changes, -EXDEV, dst, NULL);
+                        if (r != -EXDEV)
+                                return r;
                         return log_debug_errno(SYNTHETIC_ERRNO(EXDEV),
                                                "Invalid unit name component \"%s\" in alias.", dir);
                 }
@@ -1891,7 +1897,10 @@ int unit_file_verify_alias(
                 if (r < 0)
                         return log_error_errno(r, "Failed to verify alias validity: %m");
                 if (r == 0) {
-                        install_changes_add(changes, n_changes, -EXDEV, dst, info->name);
+                        r = install_changes_add(changes, n_changes, -EXDEV, dst, info->name);
+                        if (r != -EXDEV)
+                                return r;
+
                         return log_debug_errno(SYNTHETIC_ERRNO(EXDEV),
                                                "Invalid unit \"%s\" symlink \"%s\".",
                                                info->name, dst);
@@ -1905,7 +1914,9 @@ int unit_file_verify_alias(
 
                         UnitNameFlags type = unit_name_to_instance(info->name, &inst);
                         if (type < 0) {
-                                install_changes_add(changes, n_changes, -EUCLEAN, info->name, NULL);
+                                r = install_changes_add(changes, n_changes, -EUCLEAN, info->name, NULL);
+                                if (r != -EUCLEAN)
+                                        return r;
                                 return log_debug_errno(type, "Failed to extract instance name from \"%s\": %m", info->name);
                         }
 
@@ -2288,10 +2299,14 @@ static int install_context_mark_for_removal(
                         }
                 } else if (r < 0) {
                         log_debug_errno(r, "Failed to find unit %s, removing name: %m", i->name);
-                        install_changes_add(changes, n_changes, r, i->path ?: i->name, NULL);
+                        int k = install_changes_add(changes, n_changes, r, i->path ?: i->name, NULL);
+                        if (k != r)
+                                return k;
                 } else if (i->install_mode == INSTALL_MODE_MASKED) {
                         log_debug("Unit file %s is masked, ignoring.", i->name);
-                        install_changes_add(changes, n_changes, INSTALL_CHANGE_IS_MASKED, i->path ?: i->name, NULL);
+                        r = install_changes_add(changes, n_changes, INSTALL_CHANGE_IS_MASKED, i->path ?: i->name, NULL);
+                        if (r < 0)
+                                return r;
                         continue;
                 } else if (i->install_mode != INSTALL_MODE_REGULAR) {
                         log_debug("Unit %s has install mode %s, ignoring.",

From 64d61d19187219f128973d1ecf6339bf4f8f3a66 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 18 Jun 2024 17:09:34 +0200
Subject: [PATCH 2/2] install: shorten code a bit

This changes behaviour a bit, since we now keep track of OOM errors in
install_changes_add(). Which I'd argue is a good thing.
---
 src/shared/install.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index 70ba230ac16..08c2915fb53 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -2454,10 +2454,8 @@ int unit_file_unmask(
                         return -ENOMEM;
 
                 if (!dry_run && unlink(path) < 0) {
-                        if (errno != ENOENT) {
-                                RET_GATHER(r, -errno);
-                                install_changes_add(changes, n_changes, -errno, path, NULL);
-                        }
+                        if (errno != ENOENT)
+                                RET_GATHER(r, install_changes_add(changes, n_changes, -errno, path, NULL));
 
                         continue;
                 }