From 744bd1eadc0567563fc551ebb4f237134a51f905 Mon Sep 17 00:00:00 2001 From: Max Kellermann <max@musicpd.org> Date: Mon, 23 Dec 2019 17:12:17 +0100 Subject: [PATCH] time/ISO8601: refactor ParseTimeOfDay() to parse one by one This prepares the migration away from strptime() for Windows portability. But the real reason I'm doing this is that strptime() on Apple is buggy: strptime("14", "%H%M%S") (without separating colons) succeeds even though only the hour has been parsed. This fixes recent Travis failures in the ParseISO8601() unit test. --- NEWS | 2 ++ src/time/ISO8601.cxx | 65 ++++++++++++++++++++++++++++++++++---------- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/NEWS b/NEWS index 516148047..048a9972e 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.21.18 (not yet released) +* protocol + - work around Mac OS X bug in the ISO 8601 parser * output - alsa: fix hang bug with ALSA "null" outputs * storage diff --git a/src/time/ISO8601.cxx b/src/time/ISO8601.cxx index 42bba021f..32527d053 100644 --- a/src/time/ISO8601.cxx +++ b/src/time/ISO8601.cxx @@ -112,23 +112,58 @@ static const char * ParseTimeOfDay(const char *s, struct tm &tm, std::chrono::system_clock::duration &precision) noexcept { - const char *end; + /* this function always checks "end==s" to work around a + strptime() bug on OS X: if nothing could be parsed, + strptime() returns the input string (indicating success) + instead of nullptr (indicating error) */ - if ((end = strptime(s, "%T", &tm)) != nullptr) - precision = std::chrono::seconds(1); - else if ((end = strptime(s, "%H%M%S", &tm)) != nullptr) - /* no field separators */ - precision = std::chrono::seconds(1); - else if ((end = strptime(s, "%H%M", &tm)) != nullptr) - /* no field separators */ - precision = std::chrono::minutes(1); - else if ((end = strptime(s, "%H:%M", &tm)) != nullptr) - precision = std::chrono::minutes(1); - else if ((end = strptime(s, "%H", &tm)) != nullptr) - precision = std::chrono::hours(1); - else - return nullptr; + const char *end = strptime(s, "%H", &tm); + if (end == nullptr || end == s) + return end; + s = end; + precision = std::chrono::hours(1); + + if (*s == ':') { + /* with field separators: now a minute must follow */ + + ++s; + + end = strptime(s, "%M", &tm); + if (end == nullptr || end == s) + return nullptr; + + s = end; + precision = std::chrono::minutes(1); + + /* the "seconds" field is optional */ + if (*s != ':') + return s; + + ++s; + + end = strptime(s, "%S", &tm); + if (end == nullptr || end == s) + return nullptr; + + precision = std::chrono::seconds(1); + return end; + } + + /* without field separators */ + + end = strptime(s, "%M", &tm); + if (end == nullptr || end == s) + return s; + + s = end; + precision = std::chrono::minutes(1); + + end = strptime(s, "%S", &tm); + if (end == nullptr || end == s) + return s; + + precision = std::chrono::seconds(1); return end; }