From 459500a32cf74edca93ec79ee4cdd9dfcd20eee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 2 Oct 2018 12:31:13 +0200 Subject: [PATCH] shared/install: avoid overwriting 'r' counter with a partial result We want to store either the first error or the total number of changes in 'r'. Instead, we were overwriting this with the return value from install_info_traverse(). LGTM complained later in the loop that: > Comparison is always true because r >= 0. --- src/shared/install.c | 6 +++--- src/test/test-install-root.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 03773bd02f..ccb999998c 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1896,10 +1896,10 @@ static int install_context_apply( if (q < 0) return q; - r = install_info_traverse(scope, c, paths, i, flags, NULL); - if (r < 0) { + q = install_info_traverse(scope, c, paths, i, flags, NULL); + if (q < 0) { unit_file_changes_add(changes, n_changes, r, i->name, NULL); - return r; + return q; } /* We can attempt to process a masked unit when a different unit diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c index c0956fa4bb..73ea68f372 100644 --- a/src/test/test-install-root.c +++ b/src/test/test-install-root.c @@ -862,7 +862,7 @@ static void test_with_dropin(const char *root) { unit_file_changes_free(changes, n_changes); changes = NULL; n_changes = 0; - assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-4a.service"), &changes, &n_changes) == 1); + assert_se(unit_file_enable(UNIT_FILE_SYSTEM, 0, root, STRV_MAKE("with-dropin-4a.service"), &changes, &n_changes) == 2); assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "with-dropin-3.service", &state) >= 0 && state == UNIT_FILE_ENABLED); assert_se(n_changes == 2); assert_se(changes[0].type == UNIT_FILE_SYMLINK);