bpo-34373: Fix time.mktime() on AIX#12726

vstinner

Fix time.mktime() on AIX: fix code checking for mktime() failure
for years before 1970.
mktime(): rename variable 'buf' to 'tm'.
_PyTime_localtime(): avoid abs() which is limited to int type,
use "localtime" rather than "ctime" in the error message.
_PyTime_localtime(): EINVAL constant is now always available.
_PyTime_localtime(): initialize errno to 0 just in case if
localtime_r() doesn't set errno on error.

https://bugs.python.org/issue34373

vstinner

Fix time.mktime() error handling on AIX for year before 1970. Other changes: * mktime(): rename variable 'buf' to 'tm'. * _PyTime_localtime(): * Use "localtime" rather than "ctime" in the error message (specific to AIX). * Always initialize errno to 0 just in case if localtime_r() doesn't set errno on error. * On AIX, avoid abs() which is limited to int type. * EINVAL constant is now always available.

vstinner

@native-api: I added more comments. Is it better now?

@aixtools: Would you mind to review again my change please? (review the comments)

vstinner

This change also fix this compiler warning:

./Modules/timemodule.c:999:13: warning: 'buf.tm_year' is used uninitialized in this function [-Wuninitialized]

https://buildbot.python.org/all/#/builders/10/builds/2358

native-api

Wow. So my word can actually influence something here.
Seeing the "awaiting core review" label, I thought it would be treated as a passing remark at best.
I'll put more thought into it from now on now that I see that others actually risk relying on it!

vstinner

Don't worry. If anything goes wrong, it will be all my fault and I can handle it :-)

EGuesnet

Hi,
I have an error during regression tests with Python3.8.1 on AIX 6.1, and it seems related to this PR.
It occurs only on 64 bit. Test passes on 32 bit.

======================================================================
ERROR: test_add_file_after_2107 (test.test_zipfile.StoredTestsWithSourceFile)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/test/test_zipfile.py", line 606, in test_add_file_after_2107
    self.assertRaises(struct.error, zipfp.write, TESTFN)
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 816, in assertRaises
    return context.handle('assertRaises', args, kwargs)
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 202, in handle
    callable_obj(*args, **kwargs)
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 1739, in write
    zinfo = ZipInfo.from_file(filename, arcname,
  File "/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 523, in from_file
    mtime = time.localtime(st.st_mtime)
OverflowError: localtime argument out of range

Is the code 32 bit only, or maybe the test was not updated?

I can reproduce the behavior as follow:

$ python3.8_32
Python 3.8.1 (default, Jan 27 2020, 11:34:59) 
[GCC 8.3.0] on aix
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: timestamp out of range for platform time_t

$ python3.8_64
Python 3.8.1 (default, Jan 27 2020, 11:30:15) 
[GCC 8.3.0] on aix
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: localtime argument out of range

$ python3.7_32
Python 3.7.4 (default, Jan 15 2020, 15:50:53) 
[GCC 8.3.0] on aix6
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: timestamp out of range for platform time_t

$ python3.7_64
Python 3.7.4 (default, Jan 15 2020, 15:46:22) 
[GCC 8.3.0] on aix6
Type "help", "copyright", "credits" or "license" for more information.
>>> import time
>>> time.localtime(4325562452)
time.struct_time(tm_year=2107, tm_mon=1, tm_mday=27, tm_hour=10, tm_min=7, tm_sec=32, tm_wday=3, tm_yday=27, tm_isdst=0)

Before the PR, localtime after 2038 worked on 64 bit.

aixtools

Been awhile since I worked on this. From memory, the (initial) patch was only meant to affect the 32-bit build - as 64-bit did pass, iirc. Although, I do not recall ever encountering a failure with test_zipfile.

…

On 27/01/2020 15:08, EGuesnet wrote: Hi, I have an error during regression tests with Python3.8.1 on AIX 6.1, and it seems related to this PR. It occurs only on 64 bit. Test passes on 32 bit. ====================================================================== ERROR: test_add_file_after_2107 (test.test_zipfile.StoredTestsWithSourceFile) ---------------------------------------------------------------------- Traceback (most recent call last): File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/test/test_zipfile.py", line 606,in test_add_file_after_2107 self.assertRaises(struct.error, zipfp.write, TESTFN) File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 816,in assertRaises return context.handle('assertRaises', args, kwargs) File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/unittest/case.py", line 202,in handle callable_obj(*args,**kwargs) File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 1739,in write zinfo = ZipInfo.from_file(filename, arcname, File"/opt/freeware/src/packages/BUILD/Python-3.8.1/64bit/Lib/zipfile.py", line 523,in from_file mtime = time.localtime(st.st_mtime) OverflowError: localtime argument out of range Is the code 32 bit only, or maybe the test was not updated? ------------------------------------------------------------------------ I can reproduce the behavior as follow: $ python3.8_32 Python 3.8.1 (default, Jan 27 2020, 11:34:59) [GCC 8.3.0] on aix Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) Traceback (most recent call last): File "<stdin>", line 1, in <module> OverflowError: timestamp out of range for platform time_t $ python3.8_64 Python 3.8.1 (default, Jan 27 2020, 11:30:15) [GCC 8.3.0] on aix Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) Traceback (most recent call last): File "<stdin>", line 1, in <module> OverflowError: localtime argument out of range $ python3.7_32 Python 3.7.4 (default, Jan 15 2020, 15:50:53) [GCC 8.3.0] on aix6 Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) Traceback (most recent call last): File "<stdin>", line 1, in <module> OverflowError: timestamp out of range for platform time_t $ python3.7_64 Python 3.7.4 (default, Jan 15 2020, 15:46:22) [GCC 8.3.0] on aix6 Type "help", "copyright", "credits" or "license" for more information. >>> import time >>> time.localtime(4325562452) time.struct_time(tm_year=2107, tm_mon=1, tm_mday=27, tm_hour=10, tm_min=7, tm_sec=32, tm_wday=3, tm_yday=27, tm_isdst=0) Before the PR, localtime after 2038 worked on 64 bit. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12726?email_source=notifications&email_token=ACSZR5MN2RWVMC3IL2ENNNDQ73THJA5CNFSM4HEIKDVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ7TQGI#issuecomment-578762777>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACSZR5N3LUXRAUMYDXLKRGLQ73THJANCNFSM4HEIKDVA>.

vstinner

I have an error during regression tests with Python3.8.1 on AIX 6.1

This PR is closed. Please open a new issue at https://bugs.python.org/

the-knights-who-say-ni added the CLA signed label Apr 8, 2019

bedevere-bot added the awaiting core review label Apr 8, 2019

This was referenced Apr 8, 2019

bpo-34373: fix test_mktime and test_pthread_getcpuclickid tests on AIX #8726

Merged

bpo-31904: Add time module support for VxWorks RTOS #12305

Merged

native-api reviewed Apr 8, 2019

View reviewed changes

Add more comments on AIX mktime()

aaef958

native-api approved these changes Apr 9, 2019

View reviewed changes

vstinner merged commit 8709490 into python:master Apr 9, 2019

bedevere-bot removed the awaiting core review label Apr 9, 2019

vstinner deleted the mktime_aix branch April 9, 2019 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-34373: Fix time.mktime() on AIX#12726

bpo-34373: Fix time.mktime() on AIX#12726
vstinner merged 2 commits into
python:masterfrom
vstinner:mktime_aix

vstinner commented Apr 8, 2019 •

edited by bedevere-bot

Loading

Uh oh!

vstinner commented Apr 8, 2019

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

native-api commented Apr 9, 2019

Uh oh!

vstinner commented Apr 10, 2019

Uh oh!

EGuesnet commented Jan 27, 2020

Uh oh!

aixtools commented Jan 30, 2020 via email

Uh oh!

vstinner commented Jan 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

vstinner commented Apr 8, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Apr 8, 2019

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

vstinner commented Apr 9, 2019

Uh oh!

native-api commented Apr 9, 2019

Uh oh!

vstinner commented Apr 10, 2019

Uh oh!

EGuesnet commented Jan 27, 2020

Uh oh!

aixtools commented Jan 30, 2020 via email

Uh oh!

vstinner commented Jan 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

vstinner commented Apr 8, 2019 •

edited by bedevere-bot

Loading