Skip to content

Commit cbdf65a

Browse files
Sean-DerSean-Der
committed
Fixing test fails, no large changes mostly invalid frees and off-by-ones
1 parent f30b7fd commit cbdf65a

File tree

5 files changed

+56
-59
lines changed

5 files changed

+56
-59
lines changed

‎library.c‎

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,7 +1220,7 @@ static void array_zip_values_and_scores(RedisSock *redis_sock, zval *z_tab,
12201220
unsigned long idx;
12211221
zval *z_key_p, *z_value_p;
12221222

1223-
zend_hash_get_current_key_ex(keytable, &tablekey, &idx, 0);
1223+
zend_hash_get_current_key(keytable, &tablekey, &idx);
12241224
if((z_key_p = zend_hash_get_current_data(keytable)) == NULL) {
12251225
continue; /* this should never happen, according to the PHP people. */
12261226
}
@@ -1234,7 +1234,7 @@ static void array_zip_values_and_scores(RedisSock *redis_sock, zval *z_tab,
12341234
zend_hash_move_forward(keytable);
12351235

12361236
/* fetch again */
1237-
zend_hash_get_current_key_ex(keytable, &tablekey, &idx, 0);
1237+
zend_hash_get_current_key(keytable, &tablekey, &idx);
12381238
if((z_value_p = zend_hash_get_current_data(keytable)) == NULL) {
12391239
continue; /* this should never happen, according to the PHP people. */
12401240
}
@@ -1244,13 +1244,13 @@ static void array_zip_values_and_scores(RedisSock *redis_sock, zval *z_tab,
12441244

12451245
/* Decode the score depending on flag */
12461246
if (decode == SCORE_DECODE_INT && Z_STRLEN_P(z_value_p) > 0) {
1247-
add_assoc_long_ex(&z_ret, hkey, 1+hkey_len, atoi(hval+1));
1247+
add_assoc_long_ex(&z_ret, hkey, hkey_len, atoi(hval+1));
12481248
} else if (decode == SCORE_DECODE_DOUBLE) {
1249-
add_assoc_double_ex(&z_ret, hkey, 1+hkey_len, atof(hval));
1249+
add_assoc_double_ex(&z_ret, hkey, hkey_len, atof(hval));
12501250
} else {
12511251
zval z;
12521252
ZVAL_DUP(&z, z_value_p);
1253-
add_assoc_zval_ex(&z_ret, hkey, 1+hkey_len, &z);
1253+
add_assoc_zval_ex(&z_ret, hkey, hkey_len, &z);
12541254
}
12551255
}
12561256

@@ -1391,12 +1391,12 @@ PHP_REDIS_API void redis_string_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock
13911391
RETURN_FALSE;
13921392
}
13931393
IF_MULTI_OR_PIPELINE() {
1394-
zval *z = NULL;
1395-
if(redis_unserialize(redis_sock, response, response_len,
1396-
&z TSRMLS_CC) == 1)
1394+
zval z, *z_p;
1395+
z_p = &z;
1396+
if(redis_unserialize(redis_sock, response, response_len, &z_p) == 1)
13971397
{
13981398
efree(response);
1399-
add_next_index_zval(z_tab, z);
1399+
add_next_index_zval(z_tab, z_p);
14001400
} else {
14011401
add_next_index_stringl(z_tab, response, response_len);
14021402
}
@@ -1432,9 +1432,8 @@ redis_ping_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
14321432
IF_MULTI_OR_PIPELINE() {
14331433
add_next_index_stringl(z_tab, response, response_len);
14341434
} else {
1435-
RETURN_STRINGL(response, response_len);
1435+
RETURN_STRING(response);
14361436
}
1437-
efree(response);
14381437
}
14391438

14401439
/* Response for DEBUG object which is a formatted single line reply */
@@ -1852,7 +1851,8 @@ redis_mbulk_reply_loop(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
18521851
while(count > 0) {
18531852
line = redis_sock_read(redis_sock, &len TSRMLS_CC);
18541853
if (line != NULL) {
1855-
zval *z = NULL;
1854+
zval z, *z_p;
1855+
z_p = &z;
18561856
int unwrap;
18571857

18581858
/* We will attempt unserialization, if we're unserializing everything,
@@ -1862,9 +1862,9 @@ redis_mbulk_reply_loop(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
18621862
(unserialize == UNSERIALIZE_KEYS && count % 2 == 0) ||
18631863
(unserialize == UNSERIALIZE_VALS && count % 2 != 0);
18641864

1865-
if (unwrap && redis_unserialize(redis_sock, line, len, &z TSRMLS_CC)) {
1865+
if (unwrap && redis_unserialize(redis_sock, line, len, &z_p TSRMLS_CC)) {
18661866
efree(line);
1867-
add_next_index_zval(z_tab, z);
1867+
add_next_index_zval(z_tab, z_p);
18681868
} else {
18691869
add_next_index_stringl(z_tab, line, len);
18701870
}
@@ -1885,7 +1885,7 @@ PHP_REDIS_API int redis_mbulk_reply_assoc(INTERNAL_FUNCTION_PARAMETERS, RedisSoc
18851885
int i, numElems;
18861886
zval z_multi_result;
18871887

1888-
zval **z_keys = ctx;
1888+
zval *z_keys = ctx;
18891889

18901890
if(-1 == redis_check_eof(redis_sock, 0 TSRMLS_CC)) {
18911891
return -1;
@@ -1914,19 +1914,19 @@ PHP_REDIS_API int redis_mbulk_reply_assoc(INTERNAL_FUNCTION_PARAMETERS, RedisSoc
19141914
for(i = 0; i < numElems; ++i) {
19151915
response = redis_sock_read(redis_sock, &response_len TSRMLS_CC);
19161916
if(response != NULL) {
1917-
zval *z = NULL;
1918-
if(redis_unserialize(redis_sock, response, response_len, &z TSRMLS_CC) == 1) {
1917+
zval z, *z_p;
1918+
z_p = &z;
1919+
if(redis_unserialize(redis_sock, response, response_len, &z_p) == 1) {
19191920
efree(response);
1920-
add_assoc_zval_ex(&z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), z);
1921+
add_assoc_zval_ex(&z_multi_result, Z_STRVAL(z_keys[i]), Z_STRLEN(z_keys[i]), z_p);
19211922
} else {
1922-
add_assoc_stringl_ex(&z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), response, response_len);
1923+
add_assoc_stringl_ex(&z_multi_result, Z_STRVAL(z_keys[i]), Z_STRLEN(z_keys[i]), response, response_len);
19231924
}
19241925
} else {
1925-
add_assoc_bool_ex(&z_multi_result, Z_STRVAL_P(z_keys[i]), 1+Z_STRLEN_P(z_keys[i]), 0);
1926+
add_assoc_bool_ex(&z_multi_result, Z_STRVAL(z_keys[i]), Z_STRLEN(z_keys[i]), 0);
19261927
}
1927-
zval_dtor(z_keys[i]);
1928-
efree(z_keys[i]);
1929-
}
1928+
zval_dtor(&z_keys[i]);
1929+
}
19301930
efree(z_keys);
19311931

19321932
IF_MULTI_OR_PIPELINE() {
@@ -2018,7 +2018,7 @@ redis_serialize(RedisSock *redis_sock, zval *z, char **val, int *val_len
20182018
convert_to_string(&z_copy);
20192019
*val = Z_STRVAL_P(&z_copy);
20202020
*val_len = Z_STRLEN_P(&z_copy);
2021-
return 1;
2021+
return 0;
20222022

20232023
case REDIS_SERIALIZER_PHP:
20242024

@@ -2028,14 +2028,14 @@ redis_serialize(RedisSock *redis_sock, zval *z, char **val, int *val_len
20282028
*val_len = sstr.s->len;
20292029
PHP_VAR_SERIALIZE_DESTROY(ht);
20302030

2031-
return 1;
2031+
return 0;
20322032

20332033
case REDIS_SERIALIZER_IGBINARY:
20342034
#ifdef HAVE_REDIS_IGBINARY
20352035
if(igbinary_serialize(&val8, (size_t *)&sz, z TSRMLS_CC) == 0) {
20362036
*val = (char*)val8;
20372037
*val_len = (int)sz;
2038-
return 1;
2038+
return 0;
20392039
}
20402040
#endif
20412041
return 0;
@@ -2056,16 +2056,7 @@ redis_unserialize(RedisSock* redis_sock, const char *val, int val_len,
20562056
return 0;
20572057

20582058
case REDIS_SERIALIZER_PHP:
2059-
if(!*return_value) {
2060-
// TODO Sean-Der, heap allocation
2061-
//MAKE_STD_ZVAL(*return_value);
2062-
rv_free = 1;
2063-
}
2064-
#if ZEND_MODULE_API_NO >= 20100000
20652059
PHP_VAR_UNSERIALIZE_INIT(var_hash);
2066-
#else
2067-
memset(&var_hash, 0, sizeof(var_hash));
2068-
#endif
20692060
if(!php_var_unserialize(*return_value, (const unsigned char**)&val,
20702061
(const unsigned char*)val + val_len, &var_hash TSRMLS_CC)) {
20712062
if(rv_free==1) efree(*return_value);
@@ -2356,7 +2347,6 @@ redis_read_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
23562347
/* Set our return value */
23572348
ZVAL_DUP(return_value, z_ret_p);
23582349
zval_dtor(z_ret_p);
2359-
efree(z_ret_p);
23602350
}
23612351

23622352
/* Success */

‎redis.c‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,7 @@ PHP_METHOD(Redis, getMultiple)
993993
char *key;
994994
int key_len, key_free;
995995
zval z_tmp;
996+
ZVAL_UNDEF(&z_tmp);
996997

997998
/* If the key isn't a string, turn it into one */
998999
if(Z_TYPE_P(z_ele) == IS_STRING) {
@@ -1001,6 +1002,7 @@ PHP_METHOD(Redis, getMultiple)
10011002
} else {
10021003
ZVAL_DUP(&z_tmp, z_ele);
10031004

1005+
convert_to_string(&z_tmp);
10041006
key = Z_STRVAL(z_tmp);
10051007
key_len = Z_STRLEN(z_tmp);
10061008
}
@@ -1859,7 +1861,7 @@ generic_mset(INTERNAL_FUNCTION_PARAMETERS, char *kw, ResultCallback fun) {
18591861
int val_free, key_free;
18601862
char buf[32];
18611863

1862-
type = zend_hash_get_current_key_ex(keytable, &key_zstr, &idx, 0);
1864+
type = zend_hash_get_current_key(keytable, &key_zstr, &idx);
18631865
if((z_value_p = zend_hash_get_current_data(keytable)) == NULL)
18641866
{
18651867
/* Should never happen, according to the PHP people */
@@ -1873,12 +1875,14 @@ generic_mset(INTERNAL_FUNCTION_PARAMETERS, char *kw, ResultCallback fun) {
18731875
// Create string representation of our index
18741876
key_len = snprintf(buf, sizeof(buf), "%ld", (long)idx);
18751877
key = (char*)buf;
1876-
} else if(key_len > 0) {
1877-
// When not an integer key, the length will include the \0
1878-
key_len--;
18791878
} else {
18801879
key_len = key_zstr->len;
18811880
key = key_zstr->val;
1881+
1882+
// When not an integer key, the length will include the \0
1883+
if (key_len > 0) {
1884+
key_len--;
1885+
}
18821886
}
18831887

18841888
if(step == 0)
@@ -2950,6 +2954,7 @@ redis_build_pubsub_cmd(RedisSock *redis_sock, char **ret, PUBSUB_TYPE type,
29502954
char *key;
29512955
int key_len, key_free;
29522956
zval z_tmp;
2957+
ZVAL_UNDEF(&z_tmp);
29532958

29542959
if(Z_TYPE_P(z_ele) == IS_STRING) {
29552960
key = Z_STRVAL_P(z_ele);

‎redis_array.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,7 @@ PHP_METHOD(RedisArray, mset)
993993
}
994994

995995
/* Grab our key */
996-
type = zend_hash_get_current_key_ex(h_keys, &key_zstr, &idx, 0);
996+
type = zend_hash_get_current_key(h_keys, &key_zstr, &idx);
997997

998998
/* If the key isn't a string, make a string representation of it */
999999
if(type != HASH_KEY_IS_STRING) {

‎redis_array_impl.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,7 @@ ra_move_zset(const char *key, int key_len, zval *z_from, zval *z_to, long ttl TS
854854
ZVAL_DOUBLE(z_zadd_args[i], Z_DVAL_P(z_score_p));
855855

856856
/* add value */
857-
switch (zend_hash_get_current_key_ex(h_zset_vals, &val, &idx, 0)) {
857+
switch (zend_hash_get_current_key(h_zset_vals, &val, &idx)) {
858858
case HASH_KEY_IS_STRING:
859859
ZVAL_STRINGL(z_zadd_args[i+1], val->val, val->len - 1); /* we have to remove 1 because it is an array key. */
860860
break;

‎redis_commands.c‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ int redis_zinter_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
591591
HashPosition ptr;
592592
smart_string cmdstr = {0};
593593
int argc = 2, keys_count;
594-
size_t agg_op_len, key_len;
594+
size_t agg_op_len = 0, key_len = 0;
595595

596596
// Parse args
597597
if(zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "sa|a!s", &key,
@@ -659,6 +659,7 @@ int redis_zinter_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
659659
char *key;
660660
int key_free, key_len;
661661
zval z_tmp;
662+
ZVAL_UNDEF(&z_tmp);
662663

663664
if(Z_TYPE_P(z_ele) == IS_STRING) {
664665
key = Z_STRVAL_P(z_ele);
@@ -968,7 +969,7 @@ int redis_key_varval_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
968969
}
969970

970971
// Make sure we at least have a key, and we can get other args
971-
z_args = emalloc(argc * sizeof(zval*));
972+
z_args = (zval *) safe_emalloc(sizeof(zval), argc, 0);
972973
if(zend_get_parameters_array(ht, argc, z_args) == FAILURE) {
973974
efree(z_args);
974975
return FAILURE;
@@ -994,7 +995,7 @@ int redis_key_varval_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
994995
for(i=1;i<argc;i++) {
995996
arg_free = redis_serialize(redis_sock, &z_args[i], &arg, &arg_len TSRMLS_CC);
996997
redis_cmd_append_sstr(&cmdstr, arg, arg_len);
997-
if(arg_free) efree(arg);
998+
//if(arg_free) efree(arg);
998999
}
9991000

10001001
// Push out values
@@ -1077,7 +1078,7 @@ static int gen_varkey_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
10771078
}
10781079

10791080
// Allocate args
1080-
z_args = emalloc(argc * sizeof(zval *));
1081+
z_args = (zval *) safe_emalloc(sizeof(zval), argc, 0);
10811082
if(zend_get_parameters_array(ht, argc, z_args)==FAILURE) {
10821083
efree(z_args);
10831084
return FAILURE;
@@ -1231,7 +1232,7 @@ int redis_set_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
12311232
zend_hash_move_forward(kt))
12321233
{
12331234
// Grab key and value
1234-
type = zend_hash_get_current_key_ex(kt, &k, &idx, 0);
1235+
type = zend_hash_get_current_key(kt, &k, &idx);
12351236
v = zend_hash_get_current_data(kt);
12361237

12371238
/* Detect PX or EX argument and validate timeout */
@@ -1248,7 +1249,7 @@ int redis_set_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
12481249

12491250
/* Expiry can't be set < 1 */
12501251
if (expire < 1) return FAILURE;
1251-
} else if (Z_TYPE_P(v) == IS_STRING && IS_EX_PX_ARG(k->val)) {
1252+
} else if (Z_TYPE_P(v) == IS_STRING && k != NULL && IS_EX_PX_ARG(k->val)) {
12521253
set_type = Z_STRVAL_P(v);
12531254
}
12541255
}
@@ -1475,7 +1476,7 @@ int redis_hmget_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
14751476
char **cmd, int *cmd_len, short *slot, void **ctx)
14761477
{
14771478
char *key;
1478-
zval *z_arr, *z_mem, **z_mems;
1479+
zval *z_arr, *z_mem, *z_mems;
14791480
int i, count, valid=0, key_free;
14801481
size_t key_len;
14811482
HashTable *ht_arr;
@@ -1501,7 +1502,7 @@ int redis_hmget_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
15011502
key_free = redis_key_prefix(redis_sock, &key, (int *) &key_len);
15021503

15031504
// Allocate memory for mems+1 so we can have a sentinel
1504-
z_mems = ecalloc(count+1, sizeof(zval*));
1505+
z_mems = (zval *) safe_emalloc(sizeof(zval), count + 1, 0);
15051506

15061507
// Iterate over our member array
15071508
for(zend_hash_internal_pointer_reset_ex(ht_arr, &ptr);
@@ -1513,8 +1514,8 @@ int redis_hmget_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
15131514
|| Z_TYPE_P(z_mem) == IS_LONG)
15141515
{
15151516
// Copy into our member array
1516-
ZVAL_DUP(z_mems[valid], z_mem);
1517-
convert_to_string(z_mems[valid]);
1517+
ZVAL_DUP(&z_mems[valid], z_mem);
1518+
convert_to_string(&z_mems[valid]);
15181519

15191520
// Increment the member count to actually send
15201521
valid++;
@@ -1530,15 +1531,15 @@ int redis_hmget_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
15301531

15311532
// Sentinel so we can free this even if it's used and then we discard
15321533
// the transaction manually or there is a transaction failure
1533-
z_mems[valid] = NULL;
1534+
ZVAL_UNDEF(&z_mems[valid]);
15341535

15351536
// Start command construction
15361537
redis_cmd_init_sstr(&cmdstr, valid+1, "HMGET", sizeof("HMGET")-1);
15371538
redis_cmd_append_sstr(&cmdstr, key, key_len);
15381539

15391540
// Iterate over members, appending as arguments
15401541
for(i=0;i<valid;i++) {
1541-
redis_cmd_append_sstr(&cmdstr, Z_STRVAL_P(z_mems[i]), Z_STRLEN_P(z_mems[i]));
1542+
redis_cmd_append_sstr(&cmdstr, Z_STRVAL(z_mems[i]), Z_STRLEN(z_mems[i]));
15421543
}
15431544

15441545
// Set our slot
@@ -2569,7 +2570,7 @@ int redis_hdel_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
25692570
}
25702571

25712572
// Grab arguments as an array
2572-
z_args = emalloc(argc * sizeof(zval*));
2573+
z_args = (zval *) safe_emalloc(sizeof(zval), argc, 0);
25732574
if(zend_get_parameters_array(ht, argc, z_args)==FAILURE) {
25742575
efree(z_args);
25752576
return FAILURE;
@@ -2618,7 +2619,7 @@ int redis_zadd_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
26182619
int argc = ZEND_NUM_ARGS(), i;
26192620
smart_string cmdstr = {0};
26202621

2621-
z_args = emalloc(argc * sizeof(zval*));
2622+
z_args = (zval *) safe_emalloc(sizeof(zval), argc, 0);
26222623
if(zend_get_parameters_array(ht, argc, z_args)==FAILURE) {
26232624
efree(z_args);
26242625
return FAILURE;
@@ -3022,8 +3023,9 @@ void redis_unserialize_handler(INTERNAL_FUNCTION_PARAMETERS,
30223023

30233024
// We only need to attempt unserialization if we have a serializer running
30243025
if(redis_sock->serializer != REDIS_SERIALIZER_NONE) {
3025-
zval *z_ret = NULL;
3026-
if(redis_unserialize(redis_sock, value, value_len, &z_ret
3026+
zval z_ret, *z_ret_p;
3027+
z_ret_p = &z_ret;
3028+
if(redis_unserialize(redis_sock, value, value_len, &z_ret_p
30273029
TSRMLS_CC) == 0)
30283030
{
30293031
// Badly formed input, throw an execption
@@ -3032,7 +3034,7 @@ void redis_unserialize_handler(INTERNAL_FUNCTION_PARAMETERS,
30323034
0 TSRMLS_CC);
30333035
RETURN_FALSE;
30343036
}
3035-
RETURN_ZVAL(z_ret, 0, 1);
3037+
RETURN_ZVAL(z_ret_p, 0, 1);
30363038
} else {
30373039
// Just return the value that was passed to us
30383040
RETURN_STRINGL(value, value_len);

0 commit comments

Comments
 (0)