From: Jan Klemkow <jan@openbsd.org>
Subject: Re: newsyslog: refactor ISO-8601 parser
To: tech@openbsd.org
Date: Tue, 12 Aug 2025 18:29:56 +0200

ping?

On Fri, Jul 25, 2025 at 07:31:47PM +0200, Jan Klemkow wrote:
> 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?
> 
> [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);
>  }
>  
>  /*-
>