Skip to content

Changes for 2.4.1 for i686 compatibility #247

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pghmcfc
Copy link
Contributor

@pghmcfc pghmcfc commented Jun 26, 2025

This PR changes a few files (mainly test files) to get the test suite to pass with linux on i686. This reflects some filter values not fitting in an i686 long integer (32 bits), and fixes for a signed 32-bit time_t. The configure script already does all of the necessary checks; it's just a case of using the results properly.

On systems such as i686 with 32-bit long integers, expressions such as

60L * 60 * 1000 * 1000 (3600000000)

overflow the maximum value of 2147483647, causing the test to fail.
This can be fixed by using a "long long" (which is used for intmax_t)
instead, e.g.

60LL * 60 * 1000 * 1000
@@ -25,7 +25,7 @@ static void test_imap_date(void)
{ "31-Dec-9999", 253402214400LL },
#endif
/* conversions to maximum values */
#if TIME_T_MAX_BITS == 31
#if TIME_T_MAX_BITS == 31 || (TIME_T_MAX_BITS == 32 && defined(TIME_T_SIGNED))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think these checks are assuming TIME_T_MAX_BITS=31 with signed 32bit time_t. Also m4/gmtime_max.m4 has:

      printf "check failed, assuming "
      i_cv_gmtime_max_time_t=31

So I'm thinking it might be better to fix the m4 test to output TIME_T_MAX_BITS=31 instead with 32bit signed time_t. Then these other checks wouldn't need to change (except tm_is_too_large() probably).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I guess that would break utc_mktime(), and there are other checks assuming TIME_T_MAX_BITS=32. Maybe just i_cv_gmtime_max_time_t=31 should be changed to 32.

Copy link
Contributor

@sirainen sirainen Jun 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also in this commented case TIME_T_MAX_BITS == 31 would never happen, so I guess that should be simply #if #if TIME_T_MAX_BITS == 31 || (TIME_T_MAX_BITS == 32 && defined(TIME_T_SIGNED)

Or no, maybe it's nice to have the 31 check after all, even if it won't practically happen..

The code and tests have some hard-coded values based on the size of
time_t but they don't take into account whether or not time_t is a
signed value.

On systems such as i686 with time_t as a signed 32-bit integer, the
maximum value is the same as an unsigned 31-bit integer. Tweak the
configure test to treat 32-bit signed as 31 bits.
@pghmcfc
Copy link
Contributor Author

pghmcfc commented Jun 28, 2025

Changed the m4 test to make it return 31 bits instead of 32 if time_t was signed, and that made most of the rest of the changes redundant.

@pghmcfc pghmcfc requested a review from sirainen July 1, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants