Index | Thread | Search

From:
Jan Klemkow <jan@openbsd.org>
Subject:
newsyslog: refactor ISO-8601 parser
To:
tech@openbsd.org
Date:
Fri, 25 Jul 2025 19:31:47 +0200

Download raw body.

Thread
Hi,

The following diff replaces the internals of parse8601() by an
equivalent call of strptime(3) to parse the time string.

The only functional difference is the 2-digit year handling:
Values in the range 69-99 refer to years 1969 to 1999 and
values in the range 00-68 refer to years 2000 to 2068.
In the old version, 69-99 also refer to the 2069 to 2099 range.

But, POSIX said [1]:
	It is expected that in a future version of this standard the
	default century inferred from a 2-digit year will change. (This
	would apply to all commands accepting a 2-digit year as input.)

So, I guess we won't run into any problems here.

ok?

bye,
Jan

[1]: https://pubs.opengroup.org/onlinepubs/9799919799/functions/strptime.html

Index: newsyslog.c
===================================================================
RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
diff -u -p -r1.117 newsyslog.c
--- newsyslog.c	24 Jul 2025 07:36:21 -0000	1.117
+++ newsyslog.c	25 Jul 2025 15:47:30 -0000
@@ -1178,76 +1178,45 @@ lstat_log(char *file, size_t size, int f
 time_t
 parse8601(char *s)
 {
-	struct tm tm, *tmp;
-	char *t;
-	long l;
-
-	tmp = localtime(&timenow);
-	tm = *tmp;
-
-	tm.tm_hour = tm.tm_min = tm.tm_sec = 0;
-
-	l = strtol(s, &t, 10);
-	if (l < 0 || l >= INT_MAX || (*t != '\0' && *t != 'T'))
-		return (-1);
-
-	/*
-	 * Now t points either to the end of the string (if no time was
-	 * provided) or to the letter `T' which separates date and time in
-	 * ISO 8601.  The pointer arithmetic is the same for either case.
-	 */
-	switch (t - s) {
-	case 8:
-		tm.tm_year = ((l / 1000000) - 19) * 100;
-		l = l % 1000000;
-	case 6:
-		tm.tm_year -= tm.tm_year % 100;
-		tm.tm_year += l / 10000;
-		l = l % 10000;
-	case 4:
-		tm.tm_mon = (l / 100) - 1;
-		l = l % 100;
-	case 2:
-		tm.tm_mday = l;
-	case 0:
-		break;
-	default:
-		return (-1);
-	}
-
-	/* sanity check */
-	if (tm.tm_year < 70 || tm.tm_mon < 0 || tm.tm_mon > 12 ||
-	    tm.tm_mday < 1 || tm.tm_mday > 31)
-		return (-1);
-
-	if (*t != '\0') {
-		s = ++t;
-		l = strtol(s, &t, 10);
-		if (l < 0 || l >= INT_MAX ||
-		    (*t != '\0' && !isspace((unsigned char)*t)))
-			return (-1);
-
-		switch (t - s) {
-		case 6:
-			tm.tm_sec = l % 100;
-			l /= 100;
-		case 4:
-			tm.tm_min = l % 100;
-			l /= 100;
-		case 2:
-			tm.tm_hour = l;
+	char		 format[16] = { 0 };
+	struct tm	*tm;
+	char		*t;
+
+	tm = localtime(&timenow);
+	tm->tm_hour = tm->tm_min = tm->tm_sec = 0;
+	t = strchr(s, 'T');
+
+	if (s != t) {
+		switch (t == NULL ? strlen(s) : t - s) {
+		case 8: strlcat(format, "%C", sizeof format);
+		case 6: strlcat(format, "%y", sizeof format);
+		case 4: strlcat(format, "%m", sizeof format);
+		case 2: strlcat(format, "%d", sizeof format);
 		case 0:
 			break;
 		default:
-			return (-1);
+			return -1;
 		}
+	}
 
-		/* sanity check */
-		if (tm.tm_sec < 0 || tm.tm_sec > 60 || tm.tm_min < 0 ||
-		    tm.tm_min > 59 || tm.tm_hour < 0 || tm.tm_hour > 23)
-			return (-1);
+	if (t != NULL) {
+		strlcat(format, "T", sizeof format);
+
+		switch (strlen(t)) {
+		case 7: strlcat(format, "%H", sizeof format);
+		case 5: strlcat(format, "%M", sizeof format);
+		case 3: strlcat(format, "%S", sizeof format);
+		case 1:
+			break;
+		default:
+			return -1;
+		}
 	}
-	return (mktime(&tm));
+
+	if (strptime(s, format, tm) == NULL)
+		return -1;
+
+	return mktime(tm);
 }
 
 /*-