gh-92869: ctypes: Add c_time_t

[thp]

This implements a simple way of using ctypes.c_time_t for 32-bit and 64-bit time_t sizes. See #92869 for a discussion on why this is useful and considerations for different platforms. This doesn't take into account platforms where time_t is defined as double or anything else that's not a signed integer.

• Issue: Add c_time_t to ctypes #92869

[ghost]

All commit authors signed the Contributor License Agreement.

[thp]

Bump after 24 days of no activity. Any takers for reviewing this PR?

[m-tmatma]

@thp
It seems azure pipeline test was failed.
You should re-trigger it.

[thp]

@thp
It seems azure pipeline test was failed.
You should re-trigger it.

Is there a way to manually re-trigger it? I don't seem to have permissions. I can of course amend the commits and force-push to indirectly re-trigger a build?

[arhadthedev]

Is there a way to manually re-trigger it

You can close and reopen the PR.

[m-tmatma]

@thp

Is there a way to manually re-trigger it? I don't seem to have permissions. I can of course amend the commits and force-push to indirectly re-trigger a build?

There is another way to re-trigger.

You can comment /AzurePipelines run or /azp run to a PR.
https://docs.microsoft.com/en-us/azure/devops/pipelines/repos/github?view=azure-devops&tabs=yaml#comment-triggers

[AA-Turner]

Seems reasonable, please add tests though.

A

[AA-Turner]

Pinging listed names from the experts index for review.

A

[thp]

Found that the ctypes documentation's "Calling functions" documentation does this (without setting restype):

>>> print(libc.time(None))  
1150640792

This uses the default return type of int, which is wrong on systems that use 64-bit time_t (and just a happy coincidence on systems that use 32-bit time_t and happen to have 32-bit int):

>>> from ctypes import *
>>> libc = CDLL("libc.so.6")
>>> libc.time
<_FuncPtr object at 0x...>
>>> libc.time.restype
<class 'ctypes.c_int'>

Updated the documentation accordingly with setting libc.time.restype to the newly-added c_time_t, even though this introduces the concept of restype and argtypes quite early, but it's probably for the best, as not doing it can result in wrong return types as seen here. Alternatively, another libc function could be used for the "Calling functions" section, and libc.time used as an example for the "Return types" section.

The libc.time.restype == c_int error will be noticeable from 2038 onwards due to 32-bit overflow, at least on systems that have 64-bit time_t and 32-bit int (all modern systems?).

[AA-Turner]

Alternatively, another libc function could be used for the "Calling functions" section, and libc.time used as an example for the "Return types" section.

I think this is a better approach. (Thanks for noticing!)

A

[thp]

Alternatively, another libc function could be used for the "Calling functions" section, and libc.time used as an example for the "Return types" section.

I think this is a better approach. (Thanks for noticing!)

Moved the time() function into the return type docs, and used rand() from libc instead (which conveniently has no arguments and returns a C int).

[thp]

Merged upstream main branch, as there was a merge conflict shown in the Github Web UI.

Review/feedback/merge appreciated.

[thp]

Bump after 21 days of no feedback.

I know this isn't high priority, but maybe someone can review/approve this. Or just any outlook like "I won't have time to look into this before after summer".

If I should stop bumping this PR, please let me know - I hope the practice of waiting at least 20 days before nagging again is a good compromise between not being too annoying but still maybe getting it in front of the right people to review.

[AA-Turner]

This looks good from my perspective, but I'm not a ctypes specialist. I've asked if a core dev can review.

A

[kumaraditya303]

LGTM

[gpshead]

apologies about the commit message, i think i misattributed primary authorship in there by misreading github. Thanks for the PR!

[AA-Turner]

To confirm this was all @thp's work! Thanks for the review & merge @gpshead.

A