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.

2 files changed, 51 insertions, 14 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)
+	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);
-	else if ((end = strptime(s, "%H", &tm)) != nullptr)
-		precision = std::chrono::hours(1);
-	else
-		return nullptr;
 
+		/* 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;
 }