Re: [bug-gnulib] PING: strftime bugs

[Paul Eggert]

Eric Blake <address@hidden> writes:

> Per the POSIX specification of <time.h>, tp->tm_yday is restricted to be
> between 0 and 365 in a compliant program, so the user passing in a
> negative tm_yday evokes unspecified behavior.  The existing behavior did
> not crash on illegal input, so why add another comparison in the
> instruction stream to format the illegal input nicely?  (Similarly to a
> couple of other modifiers that you converted to DO_SIGNED_NUMBER - only
> tm_year and tm_isdst are unrestricted in value.)

Only %j and %m are affected by this point, right?  The tradition in
(most of) the rest of strftime is to output a valid number even if the
input is out of the POSIX range (for example, %d).  I think I'd rather
stick to the more-robust tradition, even if it costs a few extra
instructions.

> The old formula was checking if the PREVIOUS year was a leap year, you
> changed it to check if the current year is leap year.

Thanks for catching that bug.  I'll revise that code as follows
(reverting back to __isleap):

            int year_adjust = 0;
            int days = iso_week_days (tp->tm_yday, tp->tm_wday);

            /* LY is a leap year if and only if (tp->tm_year +
               TM_YEAR_BASE) is a leap year, except that LY and LY - 1
               both work correctly even when (tp->tm_year +
               TM_YEAR_BASE) would overflow.  */
            int ly = (tp->tm_year
                      - (tp->tm_year < 0 ? 0 : (TM_YEAR_BASE / 400 + 1) * 400)
                      + TM_YEAR_BASE);

            if (days < 0)
              {
                /* This ISO week belongs to the previous year.  */
                year_adjust = -1;
                days = iso_week_days (tp->tm_yday + (365 + __isleap (ly - 1)),
                                      tp->tm_wday);
              }
            else
              {
                int d = iso_week_days (tp->tm_yday - (365 + __isleap (ly)),
                                       tp->tm_wday);
                if (0 <= d)
                  {
                    /* This ISO week belongs to the next year.  */
                    year_adjust = 1;
                    days = d;
                  }
              }

> I'm okay with your use of guaranteed 2's complement unsigned math
> avoiding overflow, although a comment might be helpful.

OK, I'll mention that it's modulo UINT_MAX + 1.