IndySockets / Indy

Indy - Internet Direct
https://www.indyproject.org
434 stars 147 forks source link

time_t is on windows 64-bit #490

Open mezen opened 1 year ago

mezen commented 1 year ago

Windows time_t differs from Unix time_t: the Windows version is not bit depending and always 64. In C and only on 32 bit you can use a macro to switch to 32-bit time_t, but that is not the default and not recommended.

Windows source: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/time-time32-time64

time is a wrapper for _time64 and time_t is, by default, equivalent to __time64_t. If you need to force the compiler to interpret time_t as the old 32-bit time_t, you can define _USE_32BIT_TIME_T. We don't recommend _USE_32BIT_TIME_T, because your application may fail after January 18, 2038; the use of this macro isn't allowed on 64-bit platforms.

But on Unix and Linux systems time_t is bit depended.

Unix Source: https://www.ibm.com/docs/en/ibm-mq/9.1?topic=platforms-standard-data-types-unix-linux-windows

I hope {$IFDEF WINDOWS} is the correct compiler directive for Indy.

rlebeau commented 1 year ago

I'm hesitant to approve this change.

There are some places in Indy that don't use time_t from the IdCTypes unit. For instance, in the IdStackWindows unit, IP_ADAPTER_INFO is used, which has 2 time_t members, and that time_t is defined in Delphi's Winapi.IpTypes unit as NativeInt (and manually by Indy as NativeInt when Winapi.IpTypes is not available). Whereas in the IdSSLOpenSSLHeaders unit, time_t is used in X509_VERIFY_PARAM, but that time_t is hard-coded to LONG, which is always 32bit in Windows.

Also, in C++Builder's time.h C header, time_t is defined as __int64 for Win64 and as long otherwise, and long is always 32bit on Windows. Which seems to contradict the Microsoft doc you quoted.

PatrickvL commented 1 year ago

Perhaps these Delphi declarations initially didn't follow the Microsoft convention, or weren't updated correctly when the 64 bit era took off?

mezen commented 1 year ago

In fact: a coworker of me found this issue while he was trying to use the OpenSSL method ASN1_TIME_set in a 32-bit application. With a 32-bit time_t the function always failed, OpenSSL was expecting a 64-bit time_t.

Another hint is the RTL function DateTimeToUnix. Unix uses a time_t for that value and the definition in Delphi is int64.

And more interesting: MSDN says about IP_ADAPTER_INFO in the remarks section:

When using Visual Studio 2005 and later, the time_t datatype defaults to an 8-byte datatype, not the 4-byte datatype used for the LeaseObtained and LeaseExpires members on a 32-bit platform. To properly use the IP_ADAPTER_INFO structure on a 32-bit platform, define _USE_32BIT_TIME_T (use -D _USE_32BIT_TIME_T as an option, for example) when compiling the application to force the time_t datatype to a 4-byte datatype.

But if I try to use the API on my own, you are right, time_t has to be bit-depended (32-bit on 32-bit programs, 64-bit on 64-bit programs).

And as MSDN always says about _USE_32BIT_TIME_T: It depends on the compiler setting (if the macro is used or not). It seams so it was used in the WinAPI.

But not every C Programm is the WinAPI.

mezen commented 1 year ago

While working on the new OpenSSL 3 IO Handler I already used my own type definition (mostly alias for the existing C Types like TIdOpenSSL_Int = TIdC_INT;), so I could use a different definition there.

rlebeau commented 1 year ago

And more interesting: MSDN says about IP_ADAPTER_INFO in the remarks section:

When using Visual Studio 2005 and later, the time_t datatype defaults to an 8-byte datatype, not the 4-byte datatype used for the LeaseObtained and LeaseExpires members on a 32-bit platform. To properly use the IP_ADAPTER_INFO structure on a 32-bit platform, define _USE_32BIT_TIME_T (use -D _USE_32BIT_TIME_T as an option, for example) when compiling the application to force the time_t datatype to a 4-byte datatype.

Funny, they require those fields to be bit-dependant, so instead of just fixing the struct (ie, by using DWORD_PTR instead), they require everyone to enable a setting that has global consequences. Good job, Microsoft!