From 6369641d6f594557114b78fe740544ecf69a6d37 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Fri, 26 Feb 2016 11:25:22 +0100 Subject: clock-util: make clock_is_localtime() testable and add initial tests Add path argument to clock_is_localtime() and default to "/etc/adjtime" if it's NULL. This makes the function testable. Add test-clock: initial test cases for some scenarios, using a temporary file. This also checks the behaviour with a NULL (i. e. the system's /etc/adjtime) file. --- src/basic/clock-util.c | 7 ++-- src/basic/clock-util.h | 2 +- src/core/dbus-manager.c | 2 +- src/core/main.c | 2 +- src/test/test-clock.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ src/timedate/timedated.c | 2 +- src/timesync/timesyncd.c | 2 +- 7 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 src/test/test-clock.c (limited to 'src') diff --git a/src/basic/clock-util.c b/src/basic/clock-util.c index 507e757ff0..dd6c043af9 100644 --- a/src/basic/clock-util.c +++ b/src/basic/clock-util.c @@ -69,9 +69,12 @@ int clock_set_hwclock(const struct tm *tm) { return 0; } -int clock_is_localtime(void) { +int clock_is_localtime(const char* adjtime_path) { _cleanup_fclose_ FILE *f; + if (adjtime_path == NULL) + adjtime_path = "/etc/adjtime"; + /* * The third line of adjtime is "UTC" or "LOCAL" or nothing. * # /etc/adjtime @@ -79,7 +82,7 @@ int clock_is_localtime(void) { * 0 * UTC */ - f = fopen("/etc/adjtime", "re"); + f = fopen(adjtime_path, "re"); if (f) { char line[LINE_MAX]; bool b; diff --git a/src/basic/clock-util.h b/src/basic/clock-util.h index f471f2abcf..8830cd2f38 100644 --- a/src/basic/clock-util.h +++ b/src/basic/clock-util.h @@ -21,7 +21,7 @@ #include -int clock_is_localtime(void); +int clock_is_localtime(const char* adjtime_path); int clock_set_timezone(int *min); int clock_reset_timewarp(void); int clock_get_hwclock(struct tm *tm); diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index f939196397..00372b92b4 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -139,7 +139,7 @@ static int property_get_tainted( if (access("/proc/cgroups", F_OK) < 0) e = stpcpy(e, "cgroups-missing:"); - if (clock_is_localtime() > 0) + if (clock_is_localtime(NULL) > 0) e = stpcpy(e, "local-hwclock:"); /* remove the last ':' */ diff --git a/src/core/main.c b/src/core/main.c index b4e96fd6f4..2c315930ed 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1375,7 +1375,7 @@ int main(int argc, char *argv[]) { } if (!skip_setup) { - if (clock_is_localtime() > 0) { + if (clock_is_localtime(NULL) > 0) { int min; /* diff --git a/src/test/test-clock.c b/src/test/test-clock.c new file mode 100644 index 0000000000..27f6b8cfd2 --- /dev/null +++ b/src/test/test-clock.c @@ -0,0 +1,93 @@ +/*** + This file is part of systemd. + + Copyright (C) 2016 Canonical Ltd. + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include +#include + +#include "macro.h" +#include "fileio.h" +#include "log.h" +#include "clock-util.h" + +static void test_clock_is_localtime(void) { + char adjtime[] = "/tmp/test-adjtime.XXXXXX"; + int fd; + FILE* f; + + const struct scenario { + const char* contents; + int expected_result; + } scenarios[] = { + /* adjtime configures UTC */ + {"0.0 0 0\n0\nUTC\n", 0}, + /* adjtime configures local time */ + {"0.0 0 0\n0\nLOCAL\n", 1}, + /* no final EOL */ + {"0.0 0 0\n0\nUTC", 0}, + {"0.0 0 0\n0\nLOCAL", 1}, + /* unknown value -> defaults to UTC */ + {"0.0 0 0\n0\nFOO\n", 0}, + /* gibberish */ + {"br0ken", -EIO}, + }; + + /* without an adjtime file we default to UTC */ + assert_se(clock_is_localtime("/nonexisting/adjtime") == 0); + + fd = mkostemp_safe(adjtime, O_WRONLY|O_CLOEXEC); + assert(fd > 0); + log_info("adjtime test file: %s", adjtime); + f = fdopen(fd, "w"); + assert(f); + + for (size_t i = 0; i < ELEMENTSOF(scenarios); ++i) { + log_info("scenario #%zu:, expected result %i", i, scenarios[i].expected_result); + log_info("%s", scenarios[i].contents); + rewind(f); + ftruncate(fd, 0); + assert_se(write_string_stream(f, scenarios[i].contents, false) == 0); + assert_se(clock_is_localtime(adjtime) == scenarios[i].expected_result); + } + + unlink(adjtime); +} + +/* Test with the real /etc/adjtime */ +static void test_clock_is_localtime_system(void) { + int r; + r = clock_is_localtime(NULL); + + if (access("/etc/adjtime", F_OK) == 0) { + log_info("/etc/adjtime exists, clock_is_localtime() == %i", r); + /* we cannot assert much if /etc/adjtime exists, just that we + * expect either an answer, or an EIO if the local file really + * is badly malformed. I. e. we don't expect any other error + * code or crash. */ + assert(r == 0 || r == 1 || r == -EIO); + } else + /* default is UTC if there is no /etc/adjtime */ + assert(r == 0); +} + +int main(int argc, char *argv[]) { + test_clock_is_localtime(); + test_clock_is_localtime_system(); + + return 0; +} diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 2a10135fba..55c24ac4f0 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -78,7 +78,7 @@ static int context_read_data(Context *c) { c->zone = t; t = NULL; - c->local_rtc = clock_is_localtime() > 0; + c->local_rtc = clock_is_localtime(NULL) > 0; return 0; } diff --git a/src/timesync/timesyncd.c b/src/timesync/timesyncd.c index 23e19159e0..b67d672a6a 100644 --- a/src/timesync/timesyncd.c +++ b/src/timesync/timesyncd.c @@ -122,7 +122,7 @@ int main(int argc, char *argv[]) { goto finish; } - if (clock_is_localtime() > 0) { + if (clock_is_localtime(NULL) > 0) { log_info("The system is configured to read the RTC time in the local time zone. " "This mode can not be fully supported. All system time to RTC updates are disabled."); m->rtc_local_time = true; -- cgit v1.2.3-54-g00ecf From 35f7216f968047be26a922dc09ca588ebca71bb0 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Fri, 26 Feb 2016 12:33:41 +0100 Subject: clock-util: be more tolerant in parsing /etc/adjtime As we default to "hardware clock is in UTC" if /etc/adjtime is not present, it also makes sense to have that default if /etc/adjtime contains only one or two lines. Drop the "gibberish" test case, as this was just EIO because of not containing three lines, which is already contained in other tests. clock_is_localtime() never actually validated the format of the first two lines, and there is little point in doing that. This addresses the reading half of issue #2638. --- src/basic/clock-util.c | 4 +++- src/test/test-clock.c | 16 +++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/basic/clock-util.c b/src/basic/clock-util.c index dd6c043af9..7fe8d35ea5 100644 --- a/src/basic/clock-util.c +++ b/src/basic/clock-util.c @@ -91,7 +91,8 @@ int clock_is_localtime(const char* adjtime_path) { fgets(line, sizeof(line), f) && fgets(line, sizeof(line), f); if (!b) - return -EIO; + /* less than three lines -> default to UTC */ + return 0; truncate_nl(line); return streq(line, "LOCAL"); @@ -99,6 +100,7 @@ int clock_is_localtime(const char* adjtime_path) { } else if (errno != ENOENT) return -errno; + /* adjtime not present -> default to UTC */ return 0; } diff --git a/src/test/test-clock.c b/src/test/test-clock.c index 27f6b8cfd2..92c4f79b98 100644 --- a/src/test/test-clock.c +++ b/src/test/test-clock.c @@ -41,10 +41,14 @@ static void test_clock_is_localtime(void) { /* no final EOL */ {"0.0 0 0\n0\nUTC", 0}, {"0.0 0 0\n0\nLOCAL", 1}, + /* empty value -> defaults to UTC */ + {"0.0 0 0\n0\n", 0}, /* unknown value -> defaults to UTC */ {"0.0 0 0\n0\nFOO\n", 0}, - /* gibberish */ - {"br0ken", -EIO}, + /* no third line */ + {"0.0 0 0", 0}, + {"0.0 0 0\n", 0}, + {"0.0 0 0\n0", 0}, }; /* without an adjtime file we default to UTC */ @@ -75,11 +79,9 @@ static void test_clock_is_localtime_system(void) { if (access("/etc/adjtime", F_OK) == 0) { log_info("/etc/adjtime exists, clock_is_localtime() == %i", r); - /* we cannot assert much if /etc/adjtime exists, just that we - * expect either an answer, or an EIO if the local file really - * is badly malformed. I. e. we don't expect any other error - * code or crash. */ - assert(r == 0 || r == 1 || r == -EIO); + /* if /etc/adjtime exists we expect some answer, no error or + * crash */ + assert(r == 0 || r == 1); } else /* default is UTC if there is no /etc/adjtime */ assert(r == 0); -- cgit v1.2.3-54-g00ecf From c9410dd47f4e9ba204f489dc9812a19617af020f Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Fri, 26 Feb 2016 15:54:05 +0100 Subject: timedated: be more tolerant in parsing /etc/adjtime Similarly to the previous commit, make context_write_data_local_rtc() understand /etc/adjtime files with just one or two lines, with or without a final newline. Normalize the file to the current definition in hwclock(8), in the spirit of "be liberal what you accept and strict what you produce": Add line terminators, and set the second line to "0" if missing. Fixes: #2638 --- src/timedate/timedated.c | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/timedate/timedated.c b/src/timedate/timedated.c index 55c24ac4f0..4e120564c0 100644 --- a/src/timedate/timedated.c +++ b/src/timedate/timedated.c @@ -125,30 +125,44 @@ static int context_write_data_local_rtc(Context *c) { if (!w) return -ENOMEM; } else { - char *p, *e; + char *p; + char *e = (char*) "\n"; /* default if there are not 3 lines with \n terminator */ + const char *prepend = ""; size_t a, b; - p = strchr(s, '\n'); - if (!p) - return -EIO; - - p = strchr(p+1, '\n'); - if (!p) - return -EIO; - - p++; - e = strchr(p, '\n'); - if (!e) - return -EIO; + p = strchrnul(s, '\n'); + if (*p == '\0') { + /* only one line, no \n terminator */ + prepend = "\n0\n"; + } else if (p[1] == '\0') { + /* only one line, with \n terminator */ + ++p; + prepend = "0\n"; + } else { + p = strchr(p+1, '\n'); + if (!p) { + /* only two lines, no \n terminator */ + prepend = "\n"; + p = s + strlen(s); + } else { + char *end; + /* third line might have a \n terminator or not */ + p++; + end = strchr(p, '\n'); + /* if we actually have a fourth line, use that as suffix "e", otherwise the default \n */ + if (end) + e = end; + } + } a = p - s; b = strlen(e); - w = new(char, a + (c->local_rtc ? 5 : 3) + b + 1); + w = new(char, a + (c->local_rtc ? 5 : 3) + strlen(prepend) + b + 1); if (!w) return -ENOMEM; - *(char*) mempcpy(stpcpy(mempcpy(w, s, a), c->local_rtc ? "LOCAL" : "UTC"), e, b) = 0; + *(char*) mempcpy(stpcpy(stpcpy(mempcpy(w, s, a), prepend), c->local_rtc ? "LOCAL" : "UTC"), e, b) = 0; if (streq(w, NULL_ADJTIME_UTC)) { if (unlink("/etc/adjtime") < 0) -- cgit v1.2.3-54-g00ecf