ctime.3: mktime() may modify tm_hour due to tm_isdst

Message ID 20211010105245.53896-1-alx.manpages@gmail.com
State Not applicable
Headers
Series ctime.3: mktime() may modify tm_hour due to tm_isdst |

Checks

Context Check Description
dj/TryBot-apply_patch fail Patch failed to apply to master at the time it was sent
dj/TryBot-32bit fail Patch series failed to apply

Commit Message

Alejandro Colomar Oct. 10, 2021, 10:52 a.m. UTC
  If the input DST value is the opposite of the one that mktime()
uses, which comes from the current system timezone (see tzset(3)),
mktime() will modify the hour (and if it's in a day limit, it may
carry up to modify other fields) to normalize the time to the
correct DST.

If a user wants to avoid this, it should use UTC time, which never
has DST.  For that, setenv("TZ", "", 1); sets UTC time for the
program.  After that, the program should specify that DST is not
in effect, by setting tm_isdst to 0 (or let the system guess it
with -1), since setting tm_isdst to a positive value will result
in an error, (probably) due to mktime() considering that it is
invalid to have DST enabled for UTC times.

Cc: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
---
 man3/ctime.3 | 7 +++++++
 1 file changed, 7 insertions(+)
  

Comments

Alejandro Colomar Oct. 11, 2021, 10:27 a.m. UTC | #1
Hi Michael,

Please don't apply this patch.  I'll resend with the same contents but 
with a different commit message.

On 10/10/21 12:52 PM, Alejandro Colomar wrote:
> If the input DST value is the opposite of the one that mktime()
> uses, which comes from the current system timezone (see tzset(3)),
> mktime() will modify the hour (and if it's in a day limit, it may
> carry up to modify other fields) to normalize the time to the
> correct DST.
> 
> If a user wants to avoid this, it should use UTC time, which never
> has DST.  For that, setenv("TZ", "", 1); sets UTC time for the
> program.  After that, the program should specify that DST is not
> in effect, by setting tm_isdst to 0 (or let the system guess it
> with -1), since setting tm_isdst to a positive value will result
> in an error, (probably) due to mktime() considering that it is
> invalid to have DST enabled for UTC times.

Today I found timegm(3), which does the same as mktime(3) with TZ set to 
UTC, but doesn't require the whole program to be using UTC times, so 
I'll add this to the commit message.

BTW, timegm(3) says that you should "avoid their use" because timegm(3) 
is a Linux and BSD extension, but its use can NOT be avoided (well, it 
can, but if not done very carefully, you are likely to introduce bugs 
due to setenv(3) not being thread-safe), so I'd remove that sentence 
from timegm(3).  I think it should be in POSIX.  As FreeBSD timegm(3) 
says, there's no (safe and easy) way to roll your own timegm() based on 
standard functions:

[
      The timegm() function is not specified by any standard; its 
function can-
      not be completely emulated	using the standard functions described 
above.
]

Thanks,

Alex


> 
> Cc: Paul Eggert <eggert@cs.ucla.edu>
> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
> ---
>   man3/ctime.3 | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/man3/ctime.3 b/man3/ctime.3
> index 0e2068a09..7a5714be8 100644
> --- a/man3/ctime.3
> +++ b/man3/ctime.3
> @@ -260,6 +260,13 @@ normalized (so that, for example, 40 October is changed into 9 November);
>   is set (regardless of its initial value)
>   to a positive value or to 0, respectively,
>   to indicate whether DST is or is not in effect at the specified time.
> +If the initial value of
> +.I tm_isdst
> +is inconsistent with the one set by
> +.BR mktime (),
> +.I tm_hour
> +(and possibly other fields)
> +will be modified to normalize the time to the correct DST.
>   Calling
>   .BR mktime ()
>   also sets the external variable \fItzname\fP with
>
  
Paul Eggert Oct. 11, 2021, 3:37 p.m. UTC | #2
On 10/11/21 3:27 AM, Alejandro Colomar (man-pages) wrote:
> timegm(3) says that you should "avoid their use" because timegm(3) is a 
> Linux and BSD extension, but its use can NOT be avoided (well, it can, 
> but if not done very carefully, you are likely to introduce bugs due to 
> setenv(3) not being thread-safe), so I'd remove that sentence from 
> timegm(3).  I think it should be in POSIX.

No, NetBSD's mktime_z should be in POSIX, as it nicely generalizes both 
mktime and timegm.

mktime_z should also be in glibc, but that's another story....
  
Joseph Myers Oct. 11, 2021, 10:05 p.m. UTC | #3
On Mon, 11 Oct 2021, Paul Eggert wrote:

> On 10/11/21 3:27 AM, Alejandro Colomar (man-pages) wrote:
> > timegm(3) says that you should "avoid their use" because timegm(3) is a
> > Linux and BSD extension, but its use can NOT be avoided (well, it can, but
> > if not done very carefully, you are likely to introduce bugs due to
> > setenv(3) not being thread-safe), so I'd remove that sentence from
> > timegm(3).  I think it should be in POSIX.
> 
> No, NetBSD's mktime_z should be in POSIX, as it nicely generalizes both mktime
> and timegm.

Arguably ISO C (there's no obvious dependence on any concepts that are in 
scope of POSIX but not of ISO C), but we're now past the deadline to 
request document numbers for proposals to C23 (and while there's a 
proposal to add timegm, there's no proposal to add functions using 
explicit time zones).
  
Alejandro Colomar Oct. 15, 2021, 9:55 p.m. UTC | #4
Hi Josepf and Paul,

On 10/12/21 12:05 AM, Joseph Myers wrote:
> On Mon, 11 Oct 2021, Paul Eggert wrote:
> 
>> On 10/11/21 3:27 AM, Alejandro Colomar (man-pages) wrote:
>>> timegm(3) says that you should "avoid their use" because timegm(3) is a
>>> Linux and BSD extension, but its use can NOT be avoided (well, it can, but
>>> if not done very carefully, you are likely to introduce bugs due to
>>> setenv(3) not being thread-safe), so I'd remove that sentence from
>>> timegm(3).  I think it should be in POSIX.
>>
>> No, NetBSD's mktime_z should be in POSIX, as it nicely generalizes both mktime
>> and timegm.

Hmm, I didn´t know that one either...  Yes, it seems a nicer interface 
(and can be used to implement both mktime and timegm).

I'd still remove the warning against timegm(3) in the man page, though.

> 
> Arguably ISO C (there's no obvious dependence on any concepts that are in
> scope of POSIX but not of ISO C), but we're now past the deadline to
> request document numbers for proposals to C23 (and while there's a
> proposal to add timegm, there's no proposal to add functions using
> explicit time zones).
> 

Yes.

On 10/11/21 5:37 PM, Paul Eggert wrote:
 > mktime_z should also be in glibc, but that's another story....

BTW, I started implementing mktime_z(3) in my library, based mostly on 
glibc's mktime(3) code.  When I have something working, I'll tell you in 
case you want to pick it for glibc.

Cheers,

Alex
  

Patch

diff --git a/man3/ctime.3 b/man3/ctime.3
index 0e2068a09..7a5714be8 100644
--- a/man3/ctime.3
+++ b/man3/ctime.3
@@ -260,6 +260,13 @@  normalized (so that, for example, 40 October is changed into 9 November);
 is set (regardless of its initial value)
 to a positive value or to 0, respectively,
 to indicate whether DST is or is not in effect at the specified time.
+If the initial value of
+.I tm_isdst
+is inconsistent with the one set by
+.BR mktime (),
+.I tm_hour
+(and possibly other fields)
+will be modified to normalize the time to the correct DST.
 Calling
 .BR mktime ()
 also sets the external variable \fItzname\fP with