Skip to content

PHPC-1411: Prefer integer types for wtimeout when possible #2

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 18, 2019
Merged

PHPC-1411: Prefer integer types for wtimeout when possible #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 33 additions & 12 deletions phongo_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,18 @@
#define ADD_ASSOC_ZVAL(_zv, _key, _value) add_assoc_zval(_zv, _key, _value);
#define ADD_ASSOC_NULL_EX(_zv, _key) add_assoc_null_ex(_zv, ZEND_STRL(_key));
#define ADD_ASSOC_BOOL_EX(_zv, _key, _value) add_assoc_bool_ex(_zv, ZEND_STRL(_key), _value);
#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
do { \
zval z_int; \
php_phongo_int64_to_zval((_value), &z_int, false TSRMLS_CC); \
ADD_ASSOC_ZVAL_EX((_zv), (_key), &z_int); \
#define ZVAL_INT64_STRING(_zv, _value) \
do { \
char tmp[24]; \
int tmp_len; \
tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, (_value)); \
ZVAL_STRINGL((_zv), tmp, tmp_len); \
} while (0)
#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
do { \
zval z_int; \
ZVAL_INT64_STRING(&z_int, (_value)); \
ADD_ASSOC_ZVAL_EX((_zv), (_key), &z_int); \
} while (0)
#define ADD_NEXT_INDEX_STRINGL(_zv, _value, _len) add_next_index_stringl(_zv, _value, _len);
#define phongo_free_object_arg zend_object
Expand All @@ -113,7 +120,7 @@
0 \
} \
}
#else
#else /* PHP_VERSION_ID < 70000 */
#define phongo_char char
#define phongo_long long
#define PHONGO_LONG_FORMAT "ld"
Expand All @@ -133,13 +140,20 @@
#define ADD_ASSOC_ZVAL(_zv, _key, _value) add_assoc_zval(_zv, _key, _value);
#define ADD_ASSOC_NULL_EX(_zv, _key) add_assoc_null_ex(_zv, ZEND_STRS(_key));
#define ADD_ASSOC_BOOL_EX(_zv, _key, _value) add_assoc_bool_ex(_zv, ZEND_STRS(_key), _value);
#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
#define ZVAL_INT64_STRING(_zv, _value) \
do { \
zval* z_int; \
TSRMLS_FETCH(); \
MAKE_STD_ZVAL(z_int); \
php_phongo_int64_to_zval((_value), z_int, false TSRMLS_CC); \
ADD_ASSOC_ZVAL_EX((_zv), (_key), z_int); \
char tmp[24]; \
int tmp_len; \
tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, (_value)); \
ZVAL_STRINGL((_zv), tmp, tmp_len, 1); \
} while (0)

#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
do { \
zval* z_int; \
MAKE_STD_ZVAL(z_int); \
ZVAL_INT64_STRING(z_int, (_value)); \
ADD_ASSOC_ZVAL_EX((_zv), (_key), z_int); \
} while (0)
#define ADD_NEXT_INDEX_STRINGL(_zv, _value, _len) add_next_index_stringl(_zv, _value, _len, 1);
#define Z_PHPDATE_P(object) ((php_date_obj*) zend_object_store_get_object(object TSRMLS_CC))
Expand Down Expand Up @@ -174,6 +188,7 @@
#define ADD_INDEX_INT64(_zv, _index, _value) add_index_long((_zv), (_index), (_value))
#define ADD_NEXT_INDEX_INT64(_zv, _value) add_next_index_long((_zv), (_value))
#define ADD_ASSOC_INT64(_zv, _key, _value) add_assoc_long((_zv), (_key), (_value))
#define ZVAL_INT64(_zv, _value) ZVAL_LONG((_zv), (_value))
#elif SIZEOF_PHONGO_LONG == 4
#if PHP_VERSION_ID >= 70000
#define ADD_INDEX_INT64(_zv, _index, _value) \
Expand Down Expand Up @@ -232,6 +247,12 @@
add_assoc_long((_zv), (_key), (_value)); \
}
#endif /* PHP_VERSION_ID */
#define ZVAL_INT64(_zv, _value) \
if ((_value) > INT32_MAX || (_value) < INT32_MIN) { \
php_phongo_bson_new_int64((_zv), (_value) TSRMLS_CC); \
} else { \
ZVAL_LONG((_zv), (_value)); \
}
#else /* SIZEOF_PHONGO_LONG != 8 && SIZEOF_PHONGO_LONG != 4 */
#error Unsupported architecture (integers are neither 32-bit nor 64-bit)
#endif /* SIZEOF_PHONGO_LONG */
Expand Down
23 changes: 1 addition & 22 deletions php_phongo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1367,11 +1367,7 @@ void php_phongo_write_concern_to_zval(zval* retval, const mongoc_write_concern_t
if (wtimeout != 0) {
#if SIZEOF_LONG == 4
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
char tmp[24];
int tmp_len;

tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, wtimeout);
ADD_ASSOC_STRINGL(&retval, "wtimeout", tmp, tmp_len);
ADD_ASSOC_INT64_AS_STRING(&retval, "wtimeout", wtimeout);
} else {
ADD_ASSOC_LONG_EX(retval, "wtimeout", wtimeout);
}
Expand Down Expand Up @@ -2753,23 +2749,6 @@ void phongo_manager_init(php_phongo_manager_t* manager, const char* uri_string,
#endif
} /* }}} */

void php_phongo_int64_to_zval(const int64_t data, zval* zv, bool is_bson TSRMLS_DC) /* {{{ */
{
if (is_bson) {
php_phongo_bson_new_int64(zv, data TSRMLS_CC);
} else {
char tmp[24];
int tmp_len;

tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, data);
#if PHP_VERSION_ID >= 70000
ZVAL_STRINGL(zv, tmp, tmp_len);
#else
ZVAL_STRINGL(zv, tmp, tmp_len, 1);
#endif
}
} /* }}} */

bool php_phongo_parse_int64(int64_t* retval, const char* data, phongo_zpp_char_len data_len) /* {{{ */
{
int64_t value;
Expand Down
1 change: 0 additions & 1 deletion php_phongo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ void php_phongo_cursor_to_zval(zval* retval, const mongoc_cursor_t* cursor);
void phongo_manager_init(php_phongo_manager_t* manager, const char* uri_string, zval* options, zval* driverOptions TSRMLS_DC);
int php_phongo_set_monitoring_callbacks(mongoc_client_t* client);

void php_phongo_int64_to_zval(int64_t data, zval* zv, bool is_bson TSRMLS_DC);
bool php_phongo_parse_int64(int64_t* retval, const char* data, phongo_zpp_char_len data_len);

zend_bool phongo_writeerror_init(zval* return_value, bson_t* bson TSRMLS_DC);
Expand Down
6 changes: 3 additions & 3 deletions src/BSON/Int64.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ static PHP_METHOD(Int64, __toString)

intern = Z_INT64_OBJ_P(getThis());

php_phongo_int64_to_zval(intern->integer, return_value, false TSRMLS_CC);
ZVAL_INT64_STRING(return_value, intern->integer);
} /* }}} */

/* {{{ proto array MongoDB\BSON\Int64::jsonSerialize()
Expand Down Expand Up @@ -319,15 +319,15 @@ HashTable* php_phongo_int64_get_properties_hash(zval* object, bool is_debug TSRM
{
zval value;

php_phongo_int64_to_zval(intern->integer, &value, false TSRMLS_CC);
ZVAL_INT64_STRING(&value, intern->integer);
zend_hash_str_update(props, "integer", sizeof("integer") - 1, &value);
}
#else
{
zval* value;

MAKE_STD_ZVAL(value);
php_phongo_int64_to_zval(intern->integer, value, false TSRMLS_CC);
ZVAL_INT64_STRING(value, intern->integer);
zend_hash_update(props, "integer", sizeof("integer"), &value, sizeof(value), NULL);
}
#endif
Expand Down
6 changes: 3 additions & 3 deletions src/BSON/UTCDateTime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ static PHP_METHOD(UTCDateTime, __toString)
return;
}

php_phongo_int64_to_zval(intern->milliseconds, return_value, false TSRMLS_CC);
ZVAL_INT64_STRING(return_value, intern->milliseconds);
} /* }}} */

/* {{{ proto DateTime MongoDB\BSON\UTCDateTime::toDateTime()
Expand Down Expand Up @@ -504,15 +504,15 @@ static HashTable* php_phongo_utcdatetime_get_properties_hash(zval* object, bool
{
zval milliseconds;

php_phongo_int64_to_zval(intern->milliseconds, &milliseconds, false TSRMLS_CC);
ZVAL_INT64_STRING(&milliseconds, intern->milliseconds);
zend_hash_str_update(props, "milliseconds", sizeof("milliseconds") - 1, &milliseconds);
}
#else
{
zval* milliseconds;

MAKE_STD_ZVAL(milliseconds);
php_phongo_int64_to_zval(intern->milliseconds, milliseconds, false TSRMLS_CC);
ZVAL_INT64_STRING(milliseconds, intern->milliseconds);
zend_hash_update(props, "milliseconds", sizeof("milliseconds"), &milliseconds, sizeof(milliseconds), NULL);
}
#endif
Expand Down
42 changes: 38 additions & 4 deletions src/MongoDB/WriteConcern.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,20 @@ static HashTable* php_phongo_write_concern_get_properties_hash(zval* object, boo
if (wtimeout != 0) {
zval z_wtimeout;

php_phongo_int64_to_zval(wtimeout, &z_wtimeout, is_bson TSRMLS_CC);
if (is_bson) {
ZVAL_INT64(&z_wtimeout, wtimeout);
} else {
#if SIZEOF_LONG == 4
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
ZVAL_INT64_STRING(&z_wtimeout, wtimeout);
} else {
ZVAL_LONG(&z_wtimeout, wtimeout);
}
#else
ZVAL_LONG(&z_wtimeout, wtimeout);
#endif
}

zend_hash_str_update(props, "wtimeout", sizeof("wtimeout") - 1, &z_wtimeout);
}
#else
Expand Down Expand Up @@ -401,7 +414,20 @@ static HashTable* php_phongo_write_concern_get_properties_hash(zval* object, boo
zval* z_wtimeout;

MAKE_STD_ZVAL(z_wtimeout);
php_phongo_int64_to_zval(wtimeout, z_wtimeout, is_bson TSRMLS_CC);

if (is_bson) {
ZVAL_INT64(z_wtimeout, wtimeout);
} else {
#if SIZEOF_LONG == 4
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
ZVAL_INT64_STRING(z_wtimeout, wtimeout);
} else {
ZVAL_LONG(z_wtimeout, wtimeout);
}
#else
ZVAL_LONG(z_wtimeout, wtimeout);
#endif
}

zend_hash_update(props, "wtimeout", sizeof("wtimeout"), &z_wtimeout, sizeof(z_wtimeout), NULL);
}
Expand Down Expand Up @@ -465,7 +491,11 @@ static PHP_METHOD(WriteConcern, serialize)
}

if (wtimeout != 0) {
ADD_ASSOC_INT64_AS_STRING(&retval, "wtimeout", wtimeout);
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
ADD_ASSOC_INT64_AS_STRING(&retval, "wtimeout", wtimeout);
} else {
ADD_ASSOC_LONG_EX(&retval, "wtimeout", wtimeout);
}
}
#else
ALLOC_INIT_ZVAL(retval);
Expand All @@ -484,7 +514,11 @@ static PHP_METHOD(WriteConcern, serialize)
}

if (wtimeout != 0) {
ADD_ASSOC_INT64_AS_STRING(retval, "wtimeout", wtimeout);
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
ADD_ASSOC_INT64_AS_STRING(retval, "wtimeout", wtimeout);
} else {
ADD_ASSOC_LONG_EX(retval, "wtimeout", wtimeout);
}
}
#endif

Expand Down
22 changes: 14 additions & 8 deletions tests/manager/manager-ctor-write_concern-002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ MongoDB\Driver\Manager::__construct(): write concern options (wtimeoutms)

$tests = [
['mongodb://127.0.0.1/?wtimeoutms=1000', []],
// 64-bit wtimeout may be reported as integer or string
['mongodb://127.0.0.1/?wtimeoutms=4294967296', []],
['mongodb://127.0.0.1/?w=2&wtimeoutms=1000', []],
['mongodb://127.0.0.1/?w=majority&wtimeoutms=1000', []],
['mongodb://127.0.0.1/?w=customTagSet&wtimeoutms=1000', []],
Expand All @@ -27,46 +29,50 @@ foreach ($tests as $test) {
--EXPECTF--
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["wtimeout"]=>
%rint\(4294967296\)|string\(10\) "4294967296"%r
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
int(2)
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(8) "majority"
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(12) "customTagSet"
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
int(2)
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(8) "majority"
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(12) "customTagSet"
["wtimeout"]=>
string(4) "1000"
int(1000)
}
===DONE===
6 changes: 3 additions & 3 deletions tests/manager/manager-ctor-write_concern-006.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,18 @@ object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
int(2)
["wtimeout"]=>
string(10) "4294967296"
int(4294967296)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(8) "majority"
["wtimeout"]=>
string(10) "4294967296"
int(4294967296)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(12) "customTagSet"
["wtimeout"]=>
string(10) "4294967296"
int(4294967296)
}
===DONE===
10 changes: 5 additions & 5 deletions tests/manager/manager-getwriteconcern-001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,28 +61,28 @@ object(MongoDB\Driver\WriteConcern)#%d (%d) {
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
int(2)
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(8) "majority"
["wtimeout"]=>
string(4) "1000"
int(1000)
}
object(MongoDB\Driver\WriteConcern)#%d (%d) {
["w"]=>
string(12) "customTagSet"
["wtimeout"]=>
string(4) "1000"
int(1000)
}
===DONE===
Loading