[v2,1/2] ctime.3: mktime() may modify tm_hour due to tm_isdst

Message ID 20211011111237.9414-1-alx.manpages@gmail.com
State Superseded
Headers show
Series
  • [v2,1/2] ctime.3: mktime() may modify tm_hour due to tm_isdst
Related show

Commit Message

Khem Raj via Libc-alpha Oct. 11, 2021, 11:12 a.m.
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, the user probably wants to use UTC
time.  mktime(3) uses local time, so it's not suitable for that
(by default).  Consider the following solutions:

For that, the recommended solution is to use timegm(3), which
sadly is non-portable (it is present in Linux and the BSDs, but
not in POSIX).

A portable solution (untested) might be to implement your own
timegm(3):
	time_t portable_timegm(struct tm *tm)
	{
		tm->tm_isdst = 0;
		return mktime(tm) - timezone;
	}
and assuming no other thread will change the current timezone
during this call.

Another portable solution would involve setting the timezone
explicitly to UTC+0 with setenv() (see tzset(3)).  But this forces
all of the program to use UTC time, which might not be desirable.

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(+)

-- 
2.33.0

Comments

Paul Eggert Oct. 11, 2021, 3:36 p.m. | #1
On 10/11/21 4:12 AM, Alejandro Colomar wrote:
> +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.


I don't see why this change is necessary. mktime normalizes all its 
input fields: there's nothing special about tm_isdst and tm_hour.

If normalization isn't explained clearly enough elsewhere in the man 
page, that explanation should be fixed; there shouldn't be special-case 
wording that implies that this is the only special case.
Khem Raj via Libc-alpha Oct. 15, 2021, 9:49 p.m. | #2
Hi Paul,

On 10/11/21 5:36 PM, Paul Eggert wrote:
> On 10/11/21 4:12 AM, Alejandro Colomar wrote:

>> +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.

> 

> I don't see why this change is necessary. mktime normalizes all its 

> input fields: there's nothing special about tm_isdst and tm_hour.

> 

> If normalization isn't explained clearly enough elsewhere in the man 

> page, that explanation should be fixed; there shouldn't be special-case 

> wording that implies that this is the only special case.


Hmm, you're right.  I think I was misled by the wording "if structure 
members are outside their valid interval", which led me to think that 
since 08h was between 0 and 23 it shouldn´t be affected, but the 
normalization goes beyond that and interprets "valid range" as a more 
general concept so that the full time has to correspond to a valid time 
for a given timezone.

I'm not sure how to reword it.  I'll keep it with the original text, and 
keep thinking about it.

Thanks,

Alex


-- 
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/

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