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

Fix #81331: compile errors ibm_db2.c v2.1.3 on Windows #13

Merged
merged 2 commits into from
Aug 5, 2021

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 4, 2021

As of PHP 7.4.0, the fallback definition of ulong has been removed
from php-src[1]. This is usually not an issue on POSIX systems where
ulong is pre-defined, but on Windows and some macOS environments,
where it is not.

We simply replace ulong with unsigned long for best compatibility.

[1] php/php-src#3905

As of PHP 7.4.0, the fallback definition of `ulong` has been removed
from php-src[1].  This is usually not an issue on POSIX systems where
`ulong` is pre-defined, but on Windows and some macOS environments,
where it is not.

We simply replace `ulong` with `unsigned long` for best compatibility.

[1] <php/php-src#3905>
@NattyNarwhal
Copy link
Member

Hmmmm, I think these should be zend_ulong instead to be consistent, since some num_key used by ZEND_HASH_FOREACH_KEY_VAL seems to be defined like that instead.

ulong num_idx in _php_db2_parse_options seems unused in favour of one defined as zend_ulong, in fact.

These should actually be `zend_long`, not `ulong`.  `num_idx` in
`_php_db2_parse_options` is unused, so we remove the variable
altogether.
@cmb69
Copy link
Member Author

cmb69 commented Aug 5, 2021

I've just updated the PR according to your comment. Note that I can't test; actually, I cannot even build the extension.

@NattyNarwhal NattyNarwhal merged commit 7b25ec6 into php:master Aug 5, 2021
@cmb69 cmb69 deleted the cmb/ulong branch August 6, 2021 18:06
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