Re: out of range struct tm fields in strftime

[Rich Felker <dalias@libc.org>]

On Sun, Sep 20, 2015 at 09:54:50PM +0200, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz@port70.net> [2015-09-20 18:44:35 +0200]:
> > * Rich Felker <dalias@libc.org> [2015-09-20 12:36:29 -0400]:
> > > On Sun, Sep 20, 2015 at 02:44:50PM +0200, Szabolcs Nagy wrote:
> > > > out of range tm fields should not be treated as ub
> > > > as noted in the thread
> > > > http://sourceware.org/ml/libc-alpha/2015-09/msg00546.html
> > > > 
> > > > i have a patch but there might be simpler approaches
> > > 
> > > Wouldn't it be less invasive to just make some small changes like
> > > putting a U on some of the constants so that the arithmetic happens as
> > > unsigned?
> > > 
> > 
> > that might be simpler (and probably generates better
> > code for div,mod by const)
> > 
> > but for the nl_langinfo item computation the range
> > has to be limited properly (tm_wday and tm_mon are
> > affected).
> 
> implemented this approach with wday%7U and using some 0U+

>  static int week_num(const struct tm *tm)
>  {
> -	int val = (tm->tm_yday + 7 - (tm->tm_wday+6)%7) / 7;
> +	int val = (tm->tm_yday + 7U - (tm->tm_wday+6U)%7) / 7;
>  	/* If 1 Jan is just 1-3 days past Monday,
>  	 * the previous week is also in this year. */
> -	if ((tm->tm_wday - tm->tm_yday - 2 + 371) % 7 <= 2)
> +	if ((0U + tm->tm_wday - tm->tm_yday - 2 + 371) % 7 <= 2)
>  		val++;
>  	if (!val) {
>  		val = 52;
>  		/* If 31 December of prev year a Thursday,
>  		 * or Friday of a leap year, then the
>  		 * prev year has 53 weeks. */
> -		int dec31 = (tm->tm_wday - tm->tm_yday - 1 + 7) % 7;
> +		int dec31 = (0U + tm->tm_wday - tm->tm_yday - 1 + 7) % 7;

This is okay but it might (or might not) be less ugly to just reorder
the constants to avoid the 0U. Thoughts?

> @@ -57,17 +57,17 @@ const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const struct tm *
>  
>  	switch (f) {
>  	case 'a':
> -		item = ABDAY_1 + tm->tm_wday;
> +		item = ABDAY_1 + tm->tm_wday%7U;

This is going to be a significant code size increase on many archs,
and possibly significant performance cost too. I wonder if there's a
better way.

> @@ -143,10 +143,10 @@ const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const struct tm *
>  		width = 1;
>  		goto number;
>  	case 'U':
> -		val = (tm->tm_yday + 7 - tm->tm_wday) / 7;
> +		val = (tm->tm_yday + 7U - tm->tm_wday) / 7;

These look fine.
> @@ -165,7 +165,7 @@ const char *__strftime_fmt_1(char (*s)[100], size_t *l, int f, const struct tm *
>  		val = tm->tm_year % 100;
>  		goto number;
>  	case 'Y':
> -		val = tm->tm_year + 1900;
> +		val = tm->tm_year + 1900LL;

Also looks ok. Hopefully the compiler does not actually evaluate in
64-bit.

Rich