ext/bcmath: Fixed LONG_MAX

[SakiTakamachi]

One f is missing.

However, LONG_MAX is predefined in php.h, so this macro will probably never be used....
TBH, I think it's okay to delete it...

[cmb69]

It's doubtful to define it this way, since on LP64 architectures (which are likely used by most PHP builds) long is a 64bit type.

Given that LONG_MAX is a mandatory part of C99, I think dropping the fallback definitions is fine, but I'm a bit uneasy about doing it that late in the release cycle.

[SakiTakamachi]

Here is the definition in php.h.

php-src/main/php.h

Line 229 in e12188f

#define LONG_MAX 2147483647L

I don't know why it's defined as 32bit....

[cmb69]

I don't know why it's defined as 32bit....

Perhaps because nobody cared about it since 64bit support happened.

[SakiTakamachi]

oh...

In fact, if want a 64-bit value, use ZEND_LONG_MAX.

It seems difficult to immediately switch LONG_MAX/LONG_MIN to 64-bit, since we need to check the intention of the places where they are used.

[nielsdos]

I don't know why it's defined as 32bit....

Note that this is only when LONG_MAX is not defined in limits.h, which it should be in C99.

LONG_MAX is only used in bcmath here:

php-src/ext/bcmath/libbcmath/src/num2long.c

Lines 48 to 55 in 58aa6fc

	if (val > LONG_MAX / BASE) {
	return 0;
	}
	val *= BASE;
	if (val > LONG_MAX - n) {
	return 0;
	}

Just include php.h for the correct definition in the C file (if it's not already included indirectly), and remove the wrong definition from the header.

[cmb69]

ZEND_LONG_MAX and LONG_MAX must not be confused. For instance, on 64bit Windows, ZEND_LONG_MAX is 2**63-1, but LONG_MAX is 2**31-1.

It seems difficult to immediately switch LONG_MAX/LONG_MIN to 64-bit, […]

We cannot do this at all, because it would be wrong.

[…], since we need to check the intention of the places where they are used.

The intention of using LONG_MAX should always be the same: having the largest value that can be stored in a variable of type long.

[cmb69]

I think this is good as is for now (we can revisit this for PHP 8.5 or PHP 9.0).

Thank you for the PR!

[SakiTakamachi]

Ah, I see, I remembered incorrectly about LONG_MAX.
Thanks!

[cmb69]

Seems we had some overlap here. :)

@nielsdos, the definition in php.h is also "wrong":

php-src/main/php.h

Line 229 in e12188f

#define LONG_MAX 2147483647L

[nielsdos]

@cmb69 limits.h is included too, so the fallback doesn't matter and the value will be right.

[cmb69]

@nielsdos, there is no problem as it is now. bcmath.h includes zend.h which includes zend_types.h which includes zend_portability.h which includes limits.h. (Well, no problem regarding the possibly missing <limits.h> inclusion, but apparently a big mess regarding the inlcudes; maybe worse: php_bcmath.h includes bcmath.h which in turn includes php_bcmath.h.)

[nielsdos]

Right, then ig we can just drop these fallbacks in the next version.