From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: newsyslog: refactor ISO-8601 parser
To: Jan Klemkow <jan@openbsd.org>, tech@openbsd.org
Date: Mon, 15 Sep 2025 00:39:27 +0200

On Sun, Sep 14, 2025 at 06:39:24PM +0100, Stuart Henderson wrote:
> On 2025/09/14 14:57, Jan Klemkow wrote:
> > 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.
> 
> I think this approach makes sense, having fewer places in the tree where
> we might need to update 2-digit year handling in the future is useful.

Having same semantic like libc everywhere makes sense.

> > -   tmp = localtime(&timenow);
> > -   tm = *tmp;

The old code created a copy of the libc buffer.  I guess modifying
the libc buffer does not harm, but it feels a bit strange.  Could
you use "struct tm tm; localtime_r(&timenow, &tm)" to avoid writing
to libc memory?

> > > > +	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:
> 
> style(9) asks for /* FALLTHROUGH */ comments and I think that makes
> sense.

Here we have a solid block of composing format strings.  I doubt
that /* FALLTHROUGH */ increases readability.

anyway OK bluhm@