Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Review]Fix the potential 2038 issues #7466

Closed
wants to merge 1 commit into from
Closed

[Review]Fix the potential 2038 issues #7466

wants to merge 1 commit into from

Conversation

wenshan1
Copy link
Contributor

@wenshan1 wenshan1 commented Jul 21, 2021 •
edited

  1. The length of long in a 32-bit system is 32 bits, which cannot be used to save timestamps after 2038.
    Most operating systems have extended time_t to 64 bits.
  2. The return type of time() is time_t, the tv_sec type in struct timeval is time_t except windows.
    Possible loss of data caused by the following code:
    The code now.tv_sec = (long)time(NULL);
  3. This pull request don't solve the 2038 issues in windows.

This changes is to remove the operation of casting to long.

@bagder
Copy link
Member

bagder commented Jul 21, 2021

There are also the reverse situation platforms with 64 bit long and 32 bit time_t...

@bagder
Copy link
Member

bagder commented Jul 21, 2021

Did you verify this patch on a system with 32 bit long and 64 bit time_t to see that it handles dates after 2038?

@wenshan1 wenshan1 changed the title Fix the potential 2038 issues [WIP]Fix the potential 2038 issues Jul 22, 2021
@wenshan1
Copy link
Contributor Author

There are also the reverse situation platforms with 64 bit long and 32 bit time_t...

The win 64-bit use an abstract data model called LLP64 (or P64).
https://docs.microsoft.com/en-us/windows/win32/winprog64/abstract-data-models

@wenshan1 wenshan1 closed this Jul 22, 2021
@wenshan1 wenshan1 reopened this Jul 22, 2021
carenas
carenas reviewed Jul 22, 2021
View reviewed changes
src/tool_util.c Outdated Show resolved Hide resolved
@bagder
Copy link
Member

bagder commented Jul 22, 2021

There are also the reverse situation platforms with 64 bit long and 32 bit time_t...

The win 64-bit use an abstract data model called LLP64 (or P64).
https://docs.microsoft.com/en-us/windows/win32/winprog64/abstract-data-models

I was not referring to Windows. I'm pretty sure (early?) AIX versions work like I explained.

@wenshan1
Fix the potential 2038 issues
9c2f76c 
The length of long in a 32-bit system is 32 bits, which cannot be used to
save timestamps after 2038.
Most operating systems have extended time_t to 64 bits.

This changes is to replace the operation of casting to long with
casting to time_t.
@wenshan1 wenshan1 changed the title [WIP]Fix the potential 2038 issues [Review]Fix the potential 2038 issues Jul 23, 2021
@wenshan1
Copy link
Contributor Author

There are also the reverse situation platforms with 64 bit long and 32 bit time_t...

The win 64-bit use an abstract data model called LLP64 (or P64).
https://docs.microsoft.com/en-us/windows/win32/winprog64/abstract-data-models

I was not referring to Windows. I'm pretty sure (early?) AIX versions work like I explained.

AIX

If AIX is posix compatible system, I think my PR don't introduce a new issue even if the platform is 64-bit long and 32-bit time_t.

@bagder
Copy link
Member

bagder commented Jul 30, 2021

Thanks!

@bagder bagder closed this in 5b9fedb Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carenas carenas carenas left review comments

@bagder bagder bagder approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wenshan1 @bagder @carenas