Index | Thread | Search

From:
Jan Klemkow <jan@openbsd.org>
Subject:
Re: newsyslog: refactor ISO-8601 parser
To:
tech@openbsd.org
Date:
Sun, 14 Sep 2025 14:57:02 +0200

Download raw body.

Thread
ping?

On Tue, Aug 12, 2025 at 06:29:56PM +0200, Jan Klemkow wrote:
> 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);
> >  }
> >  
> >  /*-
> > 
>