Skip to content

Commit 366dcbe

Browse files
jmikolaalcaeus
jmikola
authored and
alcaeus
committed
PHPC-1411: Prefer integer types for wtimeout when possible
For serialization, 64-bit wtimeout values will always be encoded as strings for portability. Debug output and bsonSerialize() can always use integers on 64-bit platforms and can fall back to strings and Int64 objects on 32-bit platforms, respectively.
1 parent e501104 commit 366dcbe

21 files changed

+216
-130
lines changed

‎phongo_compat.h

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,18 @@
9191
#define ADD_ASSOC_ZVAL(_zv, _key, _value) add_assoc_zval(_zv, _key, _value);
9292
#define ADD_ASSOC_NULL_EX(_zv, _key) add_assoc_null_ex(_zv, ZEND_STRL(_key));
9393
#define ADD_ASSOC_BOOL_EX(_zv, _key, _value) add_assoc_bool_ex(_zv, ZEND_STRL(_key), _value);
94-
#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
95-
do { \
96-
zval z_int; \
97-
php_phongo_int64_to_zval((_value), &z_int, false TSRMLS_CC); \
98-
ADD_ASSOC_ZVAL_EX((_zv), (_key), &z_int); \
94+
#define ZVAL_INT64_STRING(_zv, _value) \
95+
do { \
96+
char tmp[24]; \
97+
int tmp_len; \
98+
tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, (_value)); \
99+
ZVAL_STRINGL((_zv), tmp, tmp_len); \
100+
} while (0)
101+
#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
102+
do { \
103+
zval z_int; \
104+
ZVAL_INT64_STRING(&z_int, (_value)); \
105+
ADD_ASSOC_ZVAL_EX((_zv), (_key), &z_int); \
99106
} while (0)
100107
#define ADD_NEXT_INDEX_STRINGL(_zv, _value, _len) add_next_index_stringl(_zv, _value, _len);
101108
#define phongo_free_object_arg zend_object
@@ -113,7 +120,7 @@
113120
0 \
114121
} \
115122
}
116-
#else
123+
#else /* PHP_VERSION_ID < 70000 */
117124
#define phongo_char char
118125
#define phongo_long long
119126
#define PHONGO_LONG_FORMAT "ld"
@@ -133,13 +140,20 @@
133140
#define ADD_ASSOC_ZVAL(_zv, _key, _value) add_assoc_zval(_zv, _key, _value);
134141
#define ADD_ASSOC_NULL_EX(_zv, _key) add_assoc_null_ex(_zv, ZEND_STRS(_key));
135142
#define ADD_ASSOC_BOOL_EX(_zv, _key, _value) add_assoc_bool_ex(_zv, ZEND_STRS(_key), _value);
136-
#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
143+
#define ZVAL_INT64_STRING(_zv, _value) \
137144
do { \
138-
zval* z_int; \
139-
TSRMLS_FETCH(); \
140-
MAKE_STD_ZVAL(z_int); \
141-
php_phongo_int64_to_zval((_value), z_int, false TSRMLS_CC); \
142-
ADD_ASSOC_ZVAL_EX((_zv), (_key), z_int); \
145+
char tmp[24]; \
146+
int tmp_len; \
147+
tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, (_value)); \
148+
ZVAL_STRINGL((_zv), tmp, tmp_len, 1); \
149+
} while (0)
150+
151+
#define ADD_ASSOC_INT64_AS_STRING(_zv, _key, _value) \
152+
do { \
153+
zval* z_int; \
154+
MAKE_STD_ZVAL(z_int); \
155+
ZVAL_INT64_STRING(z_int, (_value)); \
156+
ADD_ASSOC_ZVAL_EX((_zv), (_key), z_int); \
143157
} while (0)
144158
#define ADD_NEXT_INDEX_STRINGL(_zv, _value, _len) add_next_index_stringl(_zv, _value, _len, 1);
145159
#define Z_PHPDATE_P(object) ((php_date_obj*) zend_object_store_get_object(object TSRMLS_CC))
@@ -174,6 +188,7 @@
174188
#define ADD_INDEX_INT64(_zv, _index, _value) add_index_long((_zv), (_index), (_value))
175189
#define ADD_NEXT_INDEX_INT64(_zv, _value) add_next_index_long((_zv), (_value))
176190
#define ADD_ASSOC_INT64(_zv, _key, _value) add_assoc_long((_zv), (_key), (_value))
191+
#define ZVAL_INT64(_zv, _value) ZVAL_LONG((_zv), (_value))
177192
#elif SIZEOF_PHONGO_LONG == 4
178193
#if PHP_VERSION_ID >= 70000
179194
#define ADD_INDEX_INT64(_zv, _index, _value) \
@@ -232,6 +247,12 @@
232247
add_assoc_long((_zv), (_key), (_value)); \
233248
}
234249
#endif /* PHP_VERSION_ID */
250+
#define ZVAL_INT64(_zv, _value) \
251+
if ((_value) > INT32_MAX || (_value) < INT32_MIN) { \
252+
php_phongo_bson_new_int64((_zv), (_value) TSRMLS_CC); \
253+
} else { \
254+
ZVAL_LONG((_zv), (_value)); \
255+
}
235256
#else /* SIZEOF_PHONGO_LONG != 8 && SIZEOF_PHONGO_LONG != 4 */
236257
#error Unsupported architecture (integers are neither 32-bit nor 64-bit)
237258
#endif /* SIZEOF_PHONGO_LONG */

‎php_phongo.c

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,11 +1367,7 @@ void php_phongo_write_concern_to_zval(zval* retval, const mongoc_write_concern_t
13671367
if (wtimeout != 0) {
13681368
#if SIZEOF_LONG == 4
13691369
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
1370-
char tmp[24];
1371-
int tmp_len;
1372-
1373-
tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, wtimeout);
1374-
ADD_ASSOC_STRINGL(&retval, "wtimeout", tmp, tmp_len);
1370+
ADD_ASSOC_INT64_AS_STRING(&retval, "wtimeout", wtimeout);
13751371
} else {
13761372
ADD_ASSOC_LONG_EX(retval, "wtimeout", wtimeout);
13771373
}
@@ -2753,23 +2749,6 @@ void phongo_manager_init(php_phongo_manager_t* manager, const char* uri_string,
27532749
#endif
27542750
} /* }}} */
27552751

2756-
void php_phongo_int64_to_zval(const int64_t data, zval* zv, bool is_bson TSRMLS_DC) /* {{{ */
2757-
{
2758-
if (is_bson) {
2759-
php_phongo_bson_new_int64(zv, data TSRMLS_CC);
2760-
} else {
2761-
char tmp[24];
2762-
int tmp_len;
2763-
2764-
tmp_len = snprintf(tmp, sizeof(tmp), "%" PRId64, data);
2765-
#if PHP_VERSION_ID >= 70000
2766-
ZVAL_STRINGL(zv, tmp, tmp_len);
2767-
#else
2768-
ZVAL_STRINGL(zv, tmp, tmp_len, 1);
2769-
#endif
2770-
}
2771-
} /* }}} */
2772-
27732752
bool php_phongo_parse_int64(int64_t* retval, const char* data, phongo_zpp_char_len data_len) /* {{{ */
27742753
{
27752754
int64_t value;

‎php_phongo.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,6 @@ void php_phongo_cursor_to_zval(zval* retval, const mongoc_cursor_t* cursor);
163163
void phongo_manager_init(php_phongo_manager_t* manager, const char* uri_string, zval* options, zval* driverOptions TSRMLS_DC);
164164
int php_phongo_set_monitoring_callbacks(mongoc_client_t* client);
165165

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

169168
zend_bool phongo_writeerror_init(zval* return_value, bson_t* bson TSRMLS_DC);

‎src/BSON/Int64.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ static PHP_METHOD(Int64, __toString)
9090

9191
intern = Z_INT64_OBJ_P(getThis());
9292

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

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

322-
php_phongo_int64_to_zval(intern->integer, &value, false TSRMLS_CC);
322+
ZVAL_INT64_STRING(&value, intern->integer);
323323
zend_hash_str_update(props, "integer", sizeof("integer") - 1, &value);
324324
}
325325
#else
326326
{
327327
zval* value;
328328

329329
MAKE_STD_ZVAL(value);
330-
php_phongo_int64_to_zval(intern->integer, value, false TSRMLS_CC);
330+
ZVAL_INT64_STRING(value, intern->integer);
331331
zend_hash_update(props, "integer", sizeof("integer"), &value, sizeof(value), NULL);
332332
}
333333
#endif

‎src/BSON/UTCDateTime.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ static PHP_METHOD(UTCDateTime, __toString)
218218
return;
219219
}
220220

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

224224
/* {{{ proto DateTime MongoDB\BSON\UTCDateTime::toDateTime()
@@ -504,15 +504,15 @@ static HashTable* php_phongo_utcdatetime_get_properties_hash(zval* object, bool
504504
{
505505
zval milliseconds;
506506

507-
php_phongo_int64_to_zval(intern->milliseconds, &milliseconds, false TSRMLS_CC);
507+
ZVAL_INT64_STRING(&milliseconds, intern->milliseconds);
508508
zend_hash_str_update(props, "milliseconds", sizeof("milliseconds") - 1, &milliseconds);
509509
}
510510
#else
511511
{
512512
zval* milliseconds;
513513

514514
MAKE_STD_ZVAL(milliseconds);
515-
php_phongo_int64_to_zval(intern->milliseconds, milliseconds, false TSRMLS_CC);
515+
ZVAL_INT64_STRING(milliseconds, intern->milliseconds);
516516
zend_hash_update(props, "milliseconds", sizeof("milliseconds"), &milliseconds, sizeof(milliseconds), NULL);
517517
}
518518
#endif

‎src/MongoDB/WriteConcern.c

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,20 @@ static HashTable* php_phongo_write_concern_get_properties_hash(zval* object, boo
368368
if (wtimeout != 0) {
369369
zval z_wtimeout;
370370

371-
php_phongo_int64_to_zval(wtimeout, &z_wtimeout, is_bson TSRMLS_CC);
371+
if (is_bson) {
372+
ZVAL_INT64(&z_wtimeout, wtimeout);
373+
} else {
374+
#if SIZEOF_LONG == 4
375+
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
376+
ZVAL_INT64_STRING(&z_wtimeout, wtimeout);
377+
} else {
378+
ZVAL_LONG(&z_wtimeout, wtimeout);
379+
}
380+
#else
381+
ZVAL_LONG(&z_wtimeout, wtimeout);
382+
#endif
383+
}
384+
372385
zend_hash_str_update(props, "wtimeout", sizeof("wtimeout") - 1, &z_wtimeout);
373386
}
374387
#else
@@ -401,7 +414,20 @@ static HashTable* php_phongo_write_concern_get_properties_hash(zval* object, boo
401414
zval* z_wtimeout;
402415

403416
MAKE_STD_ZVAL(z_wtimeout);
404-
php_phongo_int64_to_zval(wtimeout, z_wtimeout, is_bson TSRMLS_CC);
417+
418+
if (is_bson) {
419+
ZVAL_INT64(z_wtimeout, wtimeout);
420+
} else {
421+
#if SIZEOF_LONG == 4
422+
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
423+
ZVAL_INT64_STRING(z_wtimeout, wtimeout);
424+
} else {
425+
ZVAL_LONG(z_wtimeout, wtimeout);
426+
}
427+
#else
428+
ZVAL_LONG(z_wtimeout, wtimeout);
429+
#endif
430+
}
405431

406432
zend_hash_update(props, "wtimeout", sizeof("wtimeout"), &z_wtimeout, sizeof(z_wtimeout), NULL);
407433
}
@@ -465,7 +491,11 @@ static PHP_METHOD(WriteConcern, serialize)
465491
}
466492

467493
if (wtimeout != 0) {
468-
ADD_ASSOC_INT64_AS_STRING(&retval, "wtimeout", wtimeout);
494+
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
495+
ADD_ASSOC_INT64_AS_STRING(&retval, "wtimeout", wtimeout);
496+
} else {
497+
ADD_ASSOC_LONG_EX(&retval, "wtimeout", wtimeout);
498+
}
469499
}
470500
#else
471501
ALLOC_INIT_ZVAL(retval);
@@ -484,7 +514,11 @@ static PHP_METHOD(WriteConcern, serialize)
484514
}
485515

486516
if (wtimeout != 0) {
487-
ADD_ASSOC_INT64_AS_STRING(retval, "wtimeout", wtimeout);
517+
if (wtimeout > INT32_MAX || wtimeout < INT32_MIN) {
518+
ADD_ASSOC_INT64_AS_STRING(retval, "wtimeout", wtimeout);
519+
} else {
520+
ADD_ASSOC_LONG_EX(retval, "wtimeout", wtimeout);
521+
}
488522
}
489523
#endif
490524

‎tests/manager/manager-ctor-write_concern-002.phpt

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ MongoDB\Driver\Manager::__construct(): write concern options (wtimeoutms)
55

66
$tests = [
77
['mongodb://127.0.0.1/?wtimeoutms=1000', []],
8+
// 64-bit wtimeout may be reported as integer or string
9+
['mongodb://127.0.0.1/?wtimeoutms=4294967296', []],
810
['mongodb://127.0.0.1/?w=2&wtimeoutms=1000', []],
911
['mongodb://127.0.0.1/?w=majority&wtimeoutms=1000', []],
1012
['mongodb://127.0.0.1/?w=customTagSet&wtimeoutms=1000', []],
@@ -27,46 +29,50 @@ foreach ($tests as $test) {
2729
--EXPECTF--
2830
object(MongoDB\Driver\WriteConcern)#%d (%d) {
2931
["wtimeout"]=>
30-
string(4) "1000"
32+
int(1000)
33+
}
34+
object(MongoDB\Driver\WriteConcern)#%d (%d) {
35+
["wtimeout"]=>
36+
%rint\(4294967296\)|string\(10\) "4294967296"%r
3137
}
3238
object(MongoDB\Driver\WriteConcern)#%d (%d) {
3339
["w"]=>
3440
int(2)
3541
["wtimeout"]=>
36-
string(4) "1000"
42+
int(1000)
3743
}
3844
object(MongoDB\Driver\WriteConcern)#%d (%d) {
3945
["w"]=>
4046
string(8) "majority"
4147
["wtimeout"]=>
42-
string(4) "1000"
48+
int(1000)
4349
}
4450
object(MongoDB\Driver\WriteConcern)#%d (%d) {
4551
["w"]=>
4652
string(12) "customTagSet"
4753
["wtimeout"]=>
48-
string(4) "1000"
54+
int(1000)
4955
}
5056
object(MongoDB\Driver\WriteConcern)#%d (%d) {
5157
["wtimeout"]=>
52-
string(4) "1000"
58+
int(1000)
5359
}
5460
object(MongoDB\Driver\WriteConcern)#%d (%d) {
5561
["w"]=>
5662
int(2)
5763
["wtimeout"]=>
58-
string(4) "1000"
64+
int(1000)
5965
}
6066
object(MongoDB\Driver\WriteConcern)#%d (%d) {
6167
["w"]=>
6268
string(8) "majority"
6369
["wtimeout"]=>
64-
string(4) "1000"
70+
int(1000)
6571
}
6672
object(MongoDB\Driver\WriteConcern)#%d (%d) {
6773
["w"]=>
6874
string(12) "customTagSet"
6975
["wtimeout"]=>
70-
string(4) "1000"
76+
int(1000)
7177
}
7278
===DONE===

‎tests/manager/manager-ctor-write_concern-006.phpt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,18 @@ object(MongoDB\Driver\WriteConcern)#%d (%d) {
2626
["w"]=>
2727
int(2)
2828
["wtimeout"]=>
29-
string(10) "4294967296"
29+
int(4294967296)
3030
}
3131
object(MongoDB\Driver\WriteConcern)#%d (%d) {
3232
["w"]=>
3333
string(8) "majority"
3434
["wtimeout"]=>
35-
string(10) "4294967296"
35+
int(4294967296)
3636
}
3737
object(MongoDB\Driver\WriteConcern)#%d (%d) {
3838
["w"]=>
3939
string(12) "customTagSet"
4040
["wtimeout"]=>
41-
string(10) "4294967296"
41+
int(4294967296)
4242
}
4343
===DONE===

‎tests/manager/manager-getwriteconcern-001.phpt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,28 +61,28 @@ object(MongoDB\Driver\WriteConcern)#%d (%d) {
6161
}
6262
object(MongoDB\Driver\WriteConcern)#%d (%d) {
6363
["wtimeout"]=>
64-
string(4) "1000"
64+
int(1000)
6565
}
6666
object(MongoDB\Driver\WriteConcern)#%d (%d) {
6767
["wtimeout"]=>
68-
string(4) "1000"
68+
int(1000)
6969
}
7070
object(MongoDB\Driver\WriteConcern)#%d (%d) {
7171
["w"]=>
7272
int(2)
7373
["wtimeout"]=>
74-
string(4) "1000"
74+
int(1000)
7575
}
7676
object(MongoDB\Driver\WriteConcern)#%d (%d) {
7777
["w"]=>
7878
string(8) "majority"
7979
["wtimeout"]=>
80-
string(4) "1000"
80+
int(1000)
8181
}
8282
object(MongoDB\Driver\WriteConcern)#%d (%d) {
8383
["w"]=>
8484
string(12) "customTagSet"
8585
["wtimeout"]=>
86-
string(4) "1000"
86+
int(1000)
8787
}
8888
===DONE===

0 commit comments

Comments
 (0)