core: fix mtime calculation of dropin files
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Wed, 3 Mar 2021 23:36:24 +0000 (00:36 +0100)
committerSalvatore Bonaccorso <carnil@debian.org>
Tue, 13 Jul 2021 17:29:24 +0000 (18:29 +0100)
Nominally, the bug was in unit_load_dropin(), which just took the last mtime
instead of calculating the maximum. But instead of adding code to wrap the
loop, this patch goes in the other direction.

All (correct) callers of config_parse() followed a very similar pattern to
calculate the maximum mtime. So let's simplify things by making config_parse()
assume that mtime is initialized and update it to the maximum. This makes all
the callers that care about mtime simpler and also fixes the issue in
unit_load_dropin().

config_parse_many_nulstr() and config_parse_many() are different, because it
makes sense to call them just once, and current ret_mtime behaviour make sense.

Fixes #17730, https://bugzilla.redhat.com/show_bug.cgi?id=1933137.

(cherry picked from commit da46a1bc3cd28ac36114002c216196dae004b05c)

Gbp-Pq: Name core-fix-mtime-calculation-of-dropin-files.patch

src/core/load-dropin.c
src/shared/conf-parser.c
src/shared/conf-parser.h

index d1c85e23bfaf1a0cc4bdd33be5bd842886dd4f7a..3bb48564ccdfd642cf0dcc6aac6c28cb8abcd349 100644 (file)
@@ -112,6 +112,7 @@ int unit_load_dropin(Unit *u) {
                         return log_oom();
         }
 
+        u->dropin_mtime = 0;
         STRV_FOREACH(f, u->dropin_paths)
                 (void) config_parse(
                                 u->id, *f, NULL,
index 35d301d9dbf2d3595f5d46667155c37f8c14a380..099c47a8fd902d598d8679b0fa700b7a84810707 100644 (file)
@@ -259,7 +259,7 @@ int config_parse(const char *unit,
                  const void *table,
                  ConfigParseFlags flags,
                  void *userdata,
-                 usec_t *ret_mtime) {
+                 usec_t *latest_mtime) {
 
         _cleanup_free_ char *section = NULL, *continuation = NULL;
         _cleanup_fclose_ FILE *ours = NULL;
@@ -271,6 +271,9 @@ int config_parse(const char *unit,
         assert(filename);
         assert(lookup);
 
+        /* latest_mtime is an input-output parameter: it will be updated if the mtime of the file we're
+         * looking at is later than the current *latest_mtime value. */
+
         if (!f) {
                 f = ours = fopen(filename, "re");
                 if (!f) {
@@ -413,8 +416,8 @@ int config_parse(const char *unit,
                 }
         }
 
-        if (ret_mtime)
-                *ret_mtime = mtime;
+        if (latest_mtime)
+                *latest_mtime = MAX(*latest_mtime, mtime);
 
         return 0;
 }
@@ -440,13 +443,9 @@ static int config_parse_many_files(
         }
 
         STRV_FOREACH(fn, files) {
-                usec_t t;
-
-                r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &t);
+                r = config_parse(NULL, *fn, NULL, sections, lookup, table, flags, userdata, &mtime);
                 if (r < 0)
                         return r;
-                if (t > mtime) /* Find the newest */
-                        mtime = t;
         }
 
         if (ret_mtime)
index f115cb23af5495a4489a11b2e16e0f031233c643..84c9bf61ad98126a94543a54921134428e414af7 100644 (file)
@@ -89,7 +89,7 @@ int config_parse(
                 const void *table,
                 ConfigParseFlags flags,
                 void *userdata,
-                usec_t *ret_mtime);         /* possibly NULL */
+                usec_t *latest_mtime);      /* input/output, possibly NULL */
 
 int config_parse_many_nulstr(
                 const char *conf_file,      /* possibly NULL */