Skip to content

Commit f30b7fd

Browse files
Sean-DerSean-Der
committed
Use heap allocated zvals instead of dynamic arrays
1 parent c687a51 commit f30b7fd

File tree

3 files changed

+95
-90
lines changed

3 files changed

+95
-90
lines changed

‎redis_array.c‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ ra_forward_call(INTERNAL_FUNCTION_PARAMETERS, RedisArray *ra, const char *cmd, i
321321
int key_len;
322322
int i;
323323
zval *redis_inst;
324-
zval z_fun;
324+
zval z_fun, **z_callargs;
325325
HashPosition pointer;
326326
HashTable *h_args;
327327

@@ -357,27 +357,27 @@ ra_forward_call(INTERNAL_FUNCTION_PARAMETERS, RedisArray *ra, const char *cmd, i
357357

358358
/* pass call through */
359359
ZVAL_STRING(&z_fun, cmd); /* method name */
360-
zval z_callargs[argc];
360+
z_callargs = emalloc(argc * sizeof(zval*));
361361

362362
/* copy args to array */
363363
for (i = 0, zend_hash_internal_pointer_reset_ex(h_args, &pointer);
364364
(zp_tmp = zend_hash_get_current_data_ex(h_args, &pointer)) != NULL;
365365
++i, zend_hash_move_forward_ex(h_args, &pointer)) {
366366

367-
ZVAL_DUP(&z_callargs[i], zp_tmp);
367+
ZVAL_DUP(z_callargs[i], zp_tmp);
368368
}
369369

370370
/* multi/exec */
371371
if(ra->z_multi_exec) {
372-
call_user_function(&redis_ce->function_table, ra->z_multi_exec, &z_fun, return_value, argc, z_callargs TSRMLS_CC);
372+
call_user_function(&redis_ce->function_table, ra->z_multi_exec, &z_fun, return_value, argc, *z_callargs TSRMLS_CC);
373373
efree(z_callargs);
374374
RETURN_ZVAL(getThis(), 1, 0);
375375
}
376376

377377
/* CALL! */
378378
if(ra->index && b_write_cmd) {
379379
/* call using discarded temp value and extract exec results after. */
380-
call_user_function(&redis_ce->function_table, redis_inst, &z_fun, &z_tmp, argc, z_callargs TSRMLS_CC);
380+
call_user_function(&redis_ce->function_table, redis_inst, &z_fun, &z_tmp, argc, *z_callargs TSRMLS_CC);
381381
zval_dtor(&z_tmp);
382382

383383
/* add keys to index. */
@@ -386,7 +386,7 @@ ra_forward_call(INTERNAL_FUNCTION_PARAMETERS, RedisArray *ra, const char *cmd, i
386386
/* call EXEC */
387387
ra_index_exec(redis_inst, return_value, 0 TSRMLS_CC);
388388
} else { /* call directly through. */
389-
call_user_function(&redis_ce->function_table, redis_inst, &z_fun, return_value, argc, z_callargs TSRMLS_CC);
389+
call_user_function(&redis_ce->function_table, redis_inst, &z_fun, return_value, argc, *z_callargs TSRMLS_CC);
390390

391391
/* check if we have an error. */
392392
if(RA_CALL_FAILED(return_value,cmd) && ra->prev && !b_write_cmd) { /* there was an error reading, try with prev ring. */
@@ -811,6 +811,7 @@ PHP_METHOD(RedisArray, mget)
811811
int *pos, argc, *argc_each;
812812
HashTable *h_keys;
813813
HashPosition pointer;
814+
zval **redis_instances, **argv;
814815

815816
/* Multi/exec support */
816817
HANDLE_MULTI_EXEC("MGET");
@@ -832,7 +833,8 @@ PHP_METHOD(RedisArray, mget)
832833
argc = zend_hash_num_elements(h_keys);
833834
pos = emalloc(argc * sizeof(int));
834835

835-
zval argv[argc], redis_instances[argc];
836+
redis_instances = ecalloc(argc, sizeof(zval*));
837+
argv = emalloc(argc * sizeof(zval*));
836838

837839
argc_each = emalloc(ra->count * sizeof(int));
838840
memset(argc_each, 0, ra->count * sizeof(int));
@@ -866,10 +868,10 @@ PHP_METHOD(RedisArray, mget)
866868
}
867869

868870
/* Find our node */
869-
redis_instances[i] = *ra_find_node(ra, key_lookup, key_len, &pos[i] TSRMLS_CC);
871+
redis_instances[i] = ra_find_node(ra, key_lookup, key_len, &pos[i] TSRMLS_CC);
870872

871873
argc_each[pos[i]]++; /* count number of keys per node */
872-
argv[i] = *data;
874+
argv[i] = data;
873875
}
874876

875877
/* prepare return value */
@@ -887,7 +889,7 @@ PHP_METHOD(RedisArray, mget)
887889
for(i = 0; i < argc; ++i) {
888890
if(pos[i] != n) continue;
889891

890-
ZVAL_DUP(&z_tmp, &argv[i]);
892+
ZVAL_DUP(&z_tmp, argv[i]);
891893
add_next_index_zval(&z_argarray, &z_tmp);
892894
}
893895

@@ -945,7 +947,7 @@ PHP_METHOD(RedisArray, mset)
945947
RedisArray *ra;
946948
int *pos, argc, *argc_each;
947949
HashTable *h_keys;
948-
zval *redis_inst;
950+
zval *redis_inst, **redis_instances, **argv;
949951
char *key, **keys, **key_free, kbuf[40];
950952
zend_string *key_zstr;
951953
unsigned int key_len, free_idx = 0;
@@ -971,7 +973,8 @@ PHP_METHOD(RedisArray, mset)
971973
keys = emalloc(argc * sizeof(char*));
972974
key_lens = emalloc(argc * sizeof(int));
973975

974-
zval argv[argc], redis_instances[argc];
976+
argv = emalloc(argc * sizeof(zval*));
977+
redis_instances = ecalloc(argc, sizeof(zval*));
975978

976979
/* Allocate an array holding the indexes of any keys that need freeing */
977980
key_free = emalloc(argc * sizeof(char*));
@@ -1002,9 +1005,9 @@ PHP_METHOD(RedisArray, mset)
10021005
key = key_zstr->val;
10031006
}
10041007

1005-
redis_instances[i] = *ra_find_node(ra, key, (int)key_len, &pos[i] TSRMLS_CC);
1008+
redis_instances[i] = ra_find_node(ra, key, (int)key_len, &pos[i] TSRMLS_CC);
10061009
argc_each[pos[i]]++; /* count number of keys per node */
1007-
argv[i] = *data;
1010+
argv[i] = data;
10081011
keys[i] = key;
10091012
key_lens[i] = (int)key_len;
10101013
}
@@ -1025,7 +1028,7 @@ PHP_METHOD(RedisArray, mset)
10251028

10261029
if(pos[i] != n) continue;
10271030

1028-
ZVAL_DUP(&z_tmp, &argv[i]);
1031+
ZVAL_DUP(&z_tmp, argv[i]);
10291032

10301033
add_assoc_zval_ex(&z_argarray, keys[i], key_lens[i] + 1, &z_tmp); /* +1 to count the \0 here */
10311034
found++;
@@ -1081,7 +1084,7 @@ PHP_METHOD(RedisArray, del)
10811084
int *pos, argc, *argc_each;
10821085
HashTable *h_keys;
10831086
HashPosition pointer;
1084-
zval *redis_inst;
1087+
zval *redis_inst, **redis_instances, **argv;;
10851088
long total = 0;
10861089
int free_zkeys = 0;
10871090

@@ -1122,7 +1125,8 @@ PHP_METHOD(RedisArray, del)
11221125
argc = zend_hash_num_elements(h_keys);
11231126
pos = emalloc(argc * sizeof(int));
11241127

1125-
zval redis_instances[argc], argv[argc];
1128+
argv = emalloc(argc * sizeof(zval*));
1129+
redis_instances = ecalloc(argc, sizeof(zval*));
11261130

11271131
argc_each = emalloc(ra->count * sizeof(int));
11281132
memset(argc_each, 0, ra->count * sizeof(int));
@@ -1138,9 +1142,9 @@ PHP_METHOD(RedisArray, del)
11381142
RETURN_FALSE;
11391143
}
11401144

1141-
redis_instances[i] = *ra_find_node(ra, Z_STRVAL_P(data), Z_STRLEN_P(data), &pos[i] TSRMLS_CC);
1145+
redis_instances[i] = ra_find_node(ra, Z_STRVAL_P(data), Z_STRLEN_P(data), &pos[i] TSRMLS_CC);
11421146
argc_each[pos[i]]++; /* count number of keys per node */
1143-
argv[i] = *data;
1147+
argv[i] = data;
11441148
}
11451149

11461150
/* calls */
@@ -1154,7 +1158,7 @@ PHP_METHOD(RedisArray, del)
11541158
for(i = 0; i < argc; ++i) {
11551159
if(pos[i] != n) continue;
11561160

1157-
ZVAL_DUP(&z_tmp, &argv[i]);
1161+
ZVAL_DUP(&z_tmp, argv[i]);
11581162

11591163
add_next_index_zval(&z_argarray, &z_tmp);
11601164
found++;

‎redis_array_impl.c‎

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -493,25 +493,25 @@ static void
493493
ra_index_change_keys(const char *cmd, zval *z_keys, zval *z_redis TSRMLS_DC) {
494494

495495
int i, argc;
496-
zval z_fun, z_ret;
496+
zval z_fun, z_ret, **z_args;
497497

498498
/* alloc */
499499
argc = 1 + zend_hash_num_elements(Z_ARRVAL_P(z_keys));
500-
zval z_args[argc];
500+
z_args = emalloc(argc * sizeof(zval*));
501501

502502
/* prepare first parameters */
503503
ZVAL_STRING(&z_fun, cmd);
504-
ZVAL_STRING(&z_args[0], PHPREDIS_INDEX_NAME);
504+
ZVAL_STRING(z_args[0], PHPREDIS_INDEX_NAME);
505505

506506
/* prepare keys */
507507
for(i = 0; i < argc - 1; ++i) {
508508
zval *zp;
509509
zp = zend_hash_index_find(Z_ARRVAL_P(z_keys), i);
510-
ZVAL_DUP(&z_args[i+1], zp);
510+
ZVAL_DUP(z_args[i+1], zp);
511511
}
512512

513513
/* run cmd */
514-
call_user_function(&redis_ce->function_table, z_redis, &z_fun, &z_ret, argc, z_args TSRMLS_CC);
514+
call_user_function(&redis_ce->function_table, z_redis, &z_fun, &z_ret, argc, *z_args TSRMLS_CC);
515515

516516
/* don't dtor z_ret, since we're returning z_redis */
517517
}
@@ -809,7 +809,7 @@ ra_expire_key(const char *key, int key_len, zval *z_to, long ttl TSRMLS_DC) {
809809
static zend_bool
810810
ra_move_zset(const char *key, int key_len, zval *z_from, zval *z_to, long ttl TSRMLS_DC) {
811811

812-
zval z_fun_zrange, z_fun_zadd, z_ret, z_ret_dest, z_args[4], *z_score_p;
812+
zval z_fun_zrange, z_fun_zadd, z_ret, z_ret_dest, z_args[4], *z_score_p, **z_zadd_args;
813813
int count;
814814
HashTable *h_zset_vals;
815815
zend_string *val;
@@ -839,7 +839,7 @@ ra_move_zset(const char *key, int key_len, zval *z_from, zval *z_to, long ttl TS
839839

840840
/* allocate argument array for ZADD */
841841
count = zend_hash_num_elements(h_zset_vals);
842-
zval z_zadd_args[ (1 + 2*count) ];
842+
z_zadd_args = emalloc((1 + 2*count) * sizeof(zval*));
843843

844844
for(i = 1, zend_hash_internal_pointer_reset(h_zset_vals);
845845
zend_hash_has_more_elements(h_zset_vals) == SUCCESS;
@@ -851,15 +851,15 @@ ra_move_zset(const char *key, int key_len, zval *z_from, zval *z_to, long ttl TS
851851

852852
/* add score */
853853
convert_to_double(z_score_p);
854-
ZVAL_DOUBLE(&z_zadd_args[i], Z_DVAL_P(z_score_p));
854+
ZVAL_DOUBLE(z_zadd_args[i], Z_DVAL_P(z_score_p));
855855

856856
/* add value */
857857
switch (zend_hash_get_current_key_ex(h_zset_vals, &val, &idx, 0)) {
858858
case HASH_KEY_IS_STRING:
859-
ZVAL_STRINGL(&z_zadd_args[i+1], val->val, val->len - 1); /* we have to remove 1 because it is an array key. */
859+
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;
861861
case HASH_KEY_IS_LONG:
862-
ZVAL_LONG(&z_zadd_args[i+1], (long)idx);
862+
ZVAL_LONG(z_zadd_args[i+1], (long)idx);
863863
break;
864864
default:
865865
return -1; /* Todo: log error */
@@ -870,15 +870,16 @@ ra_move_zset(const char *key, int key_len, zval *z_from, zval *z_to, long ttl TS
870870

871871
/* run ZADD on target */
872872
ZVAL_STRINGL(&z_fun_zadd, "ZADD", 4);
873-
ZVAL_STRINGL(&z_zadd_args[0], key, key_len);
874-
call_user_function(&redis_ce->function_table, z_to, &z_fun_zadd, &z_ret_dest, 1 + 2 * count, z_zadd_args TSRMLS_CC);
873+
ZVAL_STRINGL(z_zadd_args[0], key, key_len);
874+
call_user_function(&redis_ce->function_table, z_to, &z_fun_zadd, &z_ret_dest, 1 + 2 * count, *z_zadd_args TSRMLS_CC);
875875

876876
/* Expire if needed */
877877
ra_expire_key(key, key_len, z_to, ttl TSRMLS_CC);
878878

879879
/* cleanup */
880880
for(i = 0; i < 1 + 2 * count; ++i) {
881-
zval_dtor(&z_zadd_args[i]);
881+
zval_dtor(z_zadd_args[i]);
882+
efree(&z_zadd_args[i]);
882883
}
883884

884885
zval_dtor(&z_ret);
@@ -962,23 +963,23 @@ ra_move_collection(const char *key, int key_len, zval *z_from, zval *z_to,
962963
int list_count, const char **cmd_list,
963964
int add_count, const char **cmd_add, long ttl TSRMLS_DC) {
964965

965-
zval z_fun_retrieve, z_fun_sadd, z_ret, *z_data_p;
966+
zval z_fun_retrieve, z_fun_sadd, z_ret, *z_data_p, **z_retrieve_args, **z_sadd_args;
966967
int count, i;
967968
HashTable *h_set_vals;
968969

969970
/* run retrieval command on source */
970-
zval z_retrieve_args[1 + list_count];
971+
z_retrieve_args = emalloc((1+list_count) * sizeof(zval*));
971972
ZVAL_STRING(&z_fun_retrieve, cmd_list[0]); /* set the command */
972973

973974
/* set the key */
974-
ZVAL_STRINGL(&z_retrieve_args[0], key, key_len);
975+
ZVAL_STRINGL(z_retrieve_args[0], key, key_len);
975976

976977
/* possibly add some other args if they were provided. */
977978
for(i = 1; i < list_count; ++i) {
978-
ZVAL_STRING(&z_retrieve_args[i], cmd_list[i]);
979+
ZVAL_STRING(z_retrieve_args[i], cmd_list[i]);
979980
}
980981

981-
call_user_function(&redis_ce->function_table, z_from, &z_fun_retrieve, &z_ret, list_count, z_retrieve_args TSRMLS_CC);
982+
call_user_function(&redis_ce->function_table, z_from, &z_fun_retrieve, &z_ret, list_count, *z_retrieve_args TSRMLS_CC);
982983

983984
if(Z_TYPE(z_ret) != IS_ARRAY) { /* key not found or replaced */
984985
/* TODO: report? */
@@ -988,9 +989,9 @@ ra_move_collection(const char *key, int key_len, zval *z_from, zval *z_to,
988989
/* run SADD/RPUSH on target */
989990
h_set_vals = Z_ARRVAL(z_ret);
990991
count = zend_hash_num_elements(h_set_vals);
991-
zval z_sadd_args[1 + count];
992+
z_sadd_args = emalloc((1 + count) * sizeof(zval*));
992993
ZVAL_STRING(&z_fun_sadd, cmd_add[0]);
993-
ZVAL_STRINGL(&z_sadd_args[0], key, key_len);
994+
ZVAL_STRINGL(z_sadd_args[0], key, key_len);
994995

995996
for(i = 0, zend_hash_internal_pointer_reset(h_set_vals);
996997
zend_hash_has_more_elements(h_set_vals) == SUCCESS;
@@ -1001,21 +1002,21 @@ ra_move_collection(const char *key, int key_len, zval *z_from, zval *z_to,
10011002
}
10021003

10031004
/* add set elements */
1004-
ZVAL_DUP(&z_sadd_args[i+1], z_data_p);
1005+
ZVAL_DUP(z_sadd_args[i+1], z_data_p);
10051006
}
10061007

10071008
/* Clean up our input return value */
10081009
zval_dtor(&z_ret);
10091010

1010-
call_user_function(&redis_ce->function_table, z_to, &z_fun_sadd, &z_ret, count+1, z_sadd_args TSRMLS_CC);
1011+
call_user_function(&redis_ce->function_table, z_to, &z_fun_sadd, &z_ret, count+1, *z_sadd_args TSRMLS_CC);
10111012

10121013
/* Expire if needed */
10131014
ra_expire_key(key, key_len, z_to, ttl TSRMLS_CC);
10141015

10151016
/* cleanup */
10161017

10171018
for(i = 0; i < count; ++i) {
1018-
zval_dtor(&z_sadd_args[i + 1]);
1019+
zval_dtor(z_sadd_args[i + 1]);
10191020
}
10201021
efree(z_sadd_args);
10211022

0 commit comments

Comments
 (0)