Fix POSIX::strftime()

This fixes GH Perl#22351

The new sv_strftime_ints() API function acts consistently with regards
to daylight savings time, with POSIX-mandated behavior, which takes the
current locale into account.

POSIX::strftime() is problematic in regard to this.  This commit adds a
backwards compatibility mode to preserve its behavior that inadvertently
was changed by 86a9c18.

These functions are intended to wrap libc strftime(), including
normalizing the input values.  For example, if you pass 63 seconds as a
value, they will typically change that to be 3 seconds into the next
minute up.  In C, the mktime() function is typically called first to do
that normalization.  (I don't know what happens if plain strftime() is
called with unnormalized values.).  mktime() looks at the current
locale, determines if daylight savings time is in effect, and adjusts
accordingly.  Perl calls its own mktime-equivalent for you, eliminating
the need for explicitly calling something that would need to always be
called anyway, hence saving an extra step.

mini_mktime() is the Perl mktime-equivalent.  It is unaffected by
locales, and does not consider the possibility of there being daylight
savings time.  The problem is that libc strftime() is affected by
locale, and does consider dst.  Perl uses these two functions together,
and this inconsistency between them is bound to cause problems.

The 'isdst' parameter to POSIX::strftime() is used to indicate if
daylight savings is in effect for the time and date given by the other
parameters.  If perl used mktime() to normalize those values, it would
calculate this for itself, using the passed-in value only as a hint.
But since it uses mini_mktime(), it has to take that value as-is.
Thus, daylight savings alone, out of all the input values, is not
normalized.  This is contrary to the documentation, and I didn't know
this when I wrote the blamed commit.

It turns out that typical uses of this function, like

    POSIX::strftime('%s', localtime)
    POSIX::strftime('%s', gmtime)

cause the correct 'isdst' to be passed.  But this is a defect in the
interface that can bite you; changing it would cause cpan breakage.

I wrote the API function sv_strftime_ints() to not have these issues.
And, to workaround a defect in the POSIX definition of mktime().  It
turns out you can't tell it you don't want to adjust for dayight time,
except by changing the locale to one that doesn't have dst, such as the
"C" locale.  But this isn't a Perl-level function.

ext/POSIX/POSIX.xs

@@ -3588,7 +3588,7 @@ difftime(time1, time2)
 #     sv_setpv(TARG, ...) could be used rather than
 #     ST(0) = sv_2mortal(newSVpv(...))
 void
-strftime(fmt, sec, min, hour, mday, mon, year, wday = -1, yday = -1, isdst = -1)
+strftime(fmt, sec, min, hour, mday, mon, year, wday = -1, yday = -1, isdst = 0)
 	SV *		fmt
 	int		sec
 	int		min
@@ -3603,9 +3603,11 @@ strftime(fmt, sec, min, hour, mday, mon, year, wday = -1, yday = -1, isdst = -1)
 	{
             PERL_UNUSED_ARG(wday);
             PERL_UNUSED_ARG(yday);
-            PERL_UNUSED_ARG(isdst);
 
-            SV *sv = sv_strftime_ints(fmt, sec, min, hour, mday, mon, year, 0);
+            /* -isdst triggers backwards compatibility mode for non-zero
+             * 'isdst' */
+            SV *sv = sv_strftime_ints(fmt, sec, min, hour, mday, mon, year,
+                                           -abs(isdst));
 	    if (sv) {
                 sv = sv_2mortal(sv);
             }

ext/POSIX/lib/POSIX.pod

@@ -1872,15 +1872,19 @@ Returns the string.
 Synopsis:
 
 	strftime(fmt, sec, min, hour, mday, mon, year,
-		 wday = -1, yday = -1, isdst = -1)
+		 wday = -1, yday = -1, isdst = 0)
 
 The month (C<mon>) begins at zero,
 I<e.g.>, January is 0, not 1.  The
 year (C<year>) is given in years since 1900, I<e.g.>, the year 1995 is 95; the
 year 2001 is 101.  Consult your system's C<strftime()> manpage for details
 about these and the other arguments.
 
-The C<wday>, C<yday>, and C<isdst> parameters are all ignored.
+The C<wday> and C<yday> parameters are both ignored.  Their values are
+always determinable from the other parameters.
+
+C<isdst> should be C<1> or C<0>, depending on whether or not daylight
+savings time is in effect for the given time or not.
 
 If you want your code to be portable, your format (C<fmt>) argument
 should use only the conversion specifiers defined by the ANSI C
@@ -1895,11 +1899,10 @@ The C<Z> specifier is notoriously unportable since the names of
 timezones are non-standard. Sticking to the numeric specifiers is the
 safest route.
 
-The given arguments are made consistent as though by calling
-C<mktime()> before calling your system's C<strftime()> function,
-except that the C<isdst> value is not affected, so that the returned
-value will always be as if the locale doesn't have daylight savings
-time.
+The arguments, except for C<isdst>, are made consistent as though by
+calling C<mktime()> before calling your system's C<strftime()> function.
+To get correct results, you must set C<isdst> to be the proper value.
+When omitted, the function assumes daylight savings is not in effect.
 
 The string for Tuesday, December 12, 1995 in the C<C> locale.
 

ext/POSIX/t/time.t

@@ -9,7 +9,7 @@ use strict;
 
 use Config;
 use POSIX;
-use Test::More tests => 26;
+use Test::More tests => 27;
 
 # For the first go to UTC to avoid DST issues around the world when testing.  SUS3 says that
 # null should get you UTC, but some environments want the explicit names.
@@ -205,3 +205,12 @@ SKIP: {
     is(mktime(CORE::localtime($time)), $time, "mktime()");
     is(mktime(POSIX::localtime($time)), $time, "mktime()");
 }
+
+SKIP: {
+    skip "'%s' not implemented in strftime", 1 if $^O eq "VMS"
+                                               || $^O eq "MSWin32";
+    # Somewhat arbitrarily, put in 60 seconds of slack;  if this fails, it
+    # will likely be off by 1 hour
+    ok(abs(POSIX::strftime('%s', localtime) - time) < 60,
+       'GH #22351; pr: GH #22369');
+}

locale.c

@@ -8095,11 +8095,27 @@ The caller assumes ownership of the returned SV with a reference count of 1.
 
 C<sv_strftime_ints> takes a bunch of integer parameters that together
 completely define a given time.  It calculates the S<C<struct tm>> to pass to
-libc strftime(), and calls that function.  Setting C<isdst> to 0 causes the
-date to be calculated as if there is no daylight savings time in effect; any
-other value causes the possibility of daylight savings time to be considered.
-A positive value is a hint to the function that daylight savings is likely to
-be in effect for the passed-in time.
+libc strftime(), and calls that function.
+
+The value of C<isdst> is used as follows:
+
+=over
+
+=item 0
+
+No daylight savings time is in effect
+
+=item E<gt>0
+
+Check if daylight savings time is in effect, and adjust the results
+accordingly.
+
+=item E<lt>0
+
+This value is reserved for internal use by the L<POSIX> module for backwards
+compatibility purposes.
+
+=back
 
 The caller assumes ownership of the returned SV with a reference count of 1.
 
@@ -8166,8 +8182,15 @@ Perl_sv_strftime_ints(pTHX_ SV * fmt, int sec, int min, int hour,
     const char * locale = "C";
 #endif
 
+    /* A negative 'isdst' triggers backwards compatibility mode for
+     * POSIX::strftime(), in which 0 is always passed to ints_to_tm() so that
+     * the possibility of daylight savings time is never considered,  But, a 1
+     * is eventually passed to libc strftime() so that it returns the results
+     * it always has for a non-zero 'isdst'.  See GH #22351 */
     struct tm  mytm;
-    ints_to_tm(&mytm, locale, sec, min, hour, mday, mon, year, isdst);
+    ints_to_tm(&mytm, locale, sec, min, hour, mday, mon, year,
+                              MAX(0, isdst));
+    mytm.tm_isdst = MIN(1, abs(isdst));
     return sv_strftime_common(fmt, locale, &mytm);
 }