PHPC-544, PHPC-592: 64-bit platform detection and integer truncation #241
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jmikola
force-pushed
the
phpc-544
branch
3 times, most recently
from
f1c7be8 to
5c8e6e1
Compare
jmikola
changed the title
jmikola
changed the title
| @@ -41,20 +41,18 @@ | |||
| #include "php_phongo.h" | |||
| #include "php_bson.h" | |||
|
|
|||
|
|
|||
| #define BSON_APPEND_INT32(b,key,val) \ | |||
| bson_append_int32 (b, key, (int) strlen (key), val) | |||
There was a problem hiding this comment.
This macro wasn't used at all, and it was actually already defined in libbson.
| } | ||
|
|
||
| Test { "x": { "$numberLong": "4294967295" }} | ||
| [%s] PHONGO-BSON: WARNING > ENTRY: Integer overflow detected on your platform: 4294967295 |
There was a problem hiding this comment.
I wasn't able to create a 64-bit Windows executable due to some DLL issues, so I can't confirm wjether the expected output for this test is accurate. We may need to revise.
jmikola
force-pushed
the
phpc-544
branch
2 times, most recently
from
df3f1c6 to
f4f906e
Compare
| @@ -206,6 +182,35 @@ | |||
| # define PHP_STREAM_CONTEXT(stream) ((php_stream_context*) (stream)->context) | |||
| #endif | |||
|
|
|||
| #if SIZEOF_PHONGO_LONG == 8 | |||
| # define ADD_INDEX_INT64(zval, index, value) add_index_long(zval, index, value) | |||
| # define ADD_ASSOC_INT64(zval, key, value) add_assoc_long(zval, key, value) | |||
There was a problem hiding this comment.
These definitions were moved up to be consistent with bson.c, where we check for 64-bit platforms first. Additionally, I removed the trailing semi-colon from ADD_ASSOC_INT64()'s definition for consistency.
|
LGTM - tests work for HHVM too. |
jmikola
added a commit
that referenced
this pull request
Feb 29, 2016
jmikola
added a commit
that referenced
this pull request
Feb 29, 2016
The previous macro uses ZEND_STRL(), which calculates the string length via sizeof() and makes it suitable only for string literals. Since this code is utilized by php_phongo_bson_visit_int64(), we must rely on strlen() to compute the field name length.
Define phongo_long and SIZEOF_PHONGO_LONG in compatibility header. On PHP 7, this will ensure we use zend_long and SIZEOF_ZEND_LONG instead of relying on long and SIZEOF_LONG. Additionally, this adds an else condition to report a build error if the architecture is neither 32-bit nor 64-bit.
This ensures that we properly check for 64-bit support in PHP 7. Additionally, we relocate variable declarations and consolidate the conditionals so that "milliseconds" is only declared when used.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://jira.mongodb.org/browse/PHPC-544
https://jira.mongodb.org/browse/PHPC-592