8000 Fix other leaks; use zval_get_* where it makes code shorter. · jrtkcoder/phpredis@3a98610 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 3a98610

Browse files
TysonAndreTysonAndre
committed
Fix other leaks; use zval_get_* where it makes code shorter.
If zval_(ptr_)dtor wasn't called on the converted values later, it would be a memory leak (lasting only until the request was over). Follow up to phpredis#918
1 parent d5b7f85 commit 3a98610

File tree

3 files changed

+53
-104
lines changed

3 files changed

+53
-104
lines changed

‎redis.c

Lines changed: 25 additions & 56 deletions
< 10000 td data-grid-cell-id="diff-a1471159ac00556a439382b010c89a8507ebc6f03311ec7655cebd95dc6c978e-1003-994-1" data-selected="false" role="gridcell" style="background-color:var(--diffBlob-additionNum-bgColor, var(--diffBlob-addition-bgColor-num));text-align:center" tabindex="-1" valign="top" class="focusable-grid-cell diff-line-number position-relative left-side">994
Original file line numberDiff line numberDiff line change
@@ -987,20 +987,12 @@ PHP_METHOD(Redis, getMultiple)
987987
char *key;
988988
int key_free;
989989
size_t key_len;
990-
zval z_tmp;
991-
ZVAL_UNDEF(&z_tmp);
990+
zend_string *key_zstr;
992991

993-
/* If the key isn't a string, turn it into one */
994-
if(Z_TYPE_P(z_ele) == IS_STRING) {
995-
key = Z_STRVAL_P(z_ele);
996-
key_len = Z_STRLEN_P(z_ele);
997-
} else {
998-
ZVAL_DUP(&z_tmp, z_ele);
999-
1000-
convert_to_string(&z_tmp);
1001-
key = Z_STRVAL(z_tmp);
1002-
key_len = Z_STRLEN(z_tmp);
1003-
}
992+
/* If the key isn't a string, turn it into one. If it is, add to refcount */
993+
key_zstr = zval_get_string(z_ele);
+
key = ZSTR_VAL(key_zstr);
995+
key_len = ZSTR_LEN(key_zstr);
1004996

1005997
/* Apply key prefix if necissary */
1006998
key_free = redis_key_prefix(redis_sock, &key, &key_len);
@@ -1011,10 +1003,8 @@ PHP_METHOD(Redis, getMultiple)
10111003
/* Free our key if it was prefixed */
10121004
if(key_free) efree(key);
10131005

1014-
/* Free oour temporary ZVAL if we converted from a non-string */
1015-
if(Z_TYPE(z_tmp) != IS_UNDEF) {
1016-
zval_dtor(&z_tmp);
1017-
}
1006+
/* Decrement refcount/free temporary string. */
1007+
zend_string_release(key_zstr);
10181008
}
10191009

10201010
/* Kick off our command */
@@ -2943,19 +2933,10 @@ redis_build_pubsub_cmd(RedisSock *redis_sock, char **ret, PUBSUB_TYPE type,
29432933
char *key;
29442934
int key_free;
29452935
size_t key_len;
2946-
zval z_tmp;
2947-
ZVAL_UNDEF(&z_tmp);
2948-
2949-
if(Z_TYPE_P(z_ele) == IS_STRING) {
2950-
key = Z_STRVAL_P(z_ele);
2951-
key_len = Z_STRLEN_P(z_ele);
2952-
} else {
2953-
ZVAL_DUP(&z_tmp, z_ele);
2954-
convert_to_string(&z_tmp);
2955-
2956-
key = Z_STRVAL(z_tmp);
2957-
key_len = Z_STRLEN(z_tmp);
2958-
}
2936+
zend_string *key_zstr;
2937+
key_zstr = zval_get_string(z_ele);
2938+
key = ZSTR_VAL(key_zstr);
2939+
key_len = ZSTR_LEN(key_zstr);
29592940

29602941
/* Apply prefix if required */
29612942
key_free = redis_key_prefix(redis_sock, &key, &key_len);
@@ -2966,10 +2947,8 @@ redis_build_pubsub_cmd(RedisSock *redis_sock, char **ret, PUBSUB_TYPE type,
29662947
/* Free key if prefixed */
29672948
if(key_free) efree(key);
29682949

2969-
// Free our temp var if we used it
2970-
if(Z_TYPE(z_tmp) != IS_UNDEF) {
2971-
zval_dtor(&z_tmp);
2972-
}
2950+
/* Decrement reference count to temporary/underlying zend_string */
2951+
zend_string_release(key_zstr);
29732952
}
29742953

29752954
/* Set return */
@@ -3095,23 +3074,13 @@ redis_build_eval_cmd(RedisSock *redis_sock, char **ret, char *keyword,
30953074
(elem = zend_hash_get_current_data_ex(args_hash, &hash_pos)) != NULL;
30963075
zend_hash_move_forward_ex(args_hash, &hash_pos))
30973076
{
3098-
zval z_tmp;
30993077
char *key, *old_cmd;
31003078
int key_free;
31013079
size_t key_len;
3102-
ZVAL_UNDEF(&z_tmp);
3103-
3104-
if(Z_TYPE_P(elem) == IS_STRING) {
3105-
key = Z_STRVAL_P(elem);
3106-
key_len = Z_STRLEN_P(elem);
3107-
} else {
3108-
/* Convert it to a string */
3109-
ZVAL_DUP(&z_tmp, elem);
3110-
convert_to_string(&z_tmp);
3111-
3112-
key = Z_STRVAL(z_tmp);
3113-
key_len = Z_STRLEN(z_tmp);
3114-
}
3080+
zend_string *key_zstr;
3081+
key_zstr = zval_get_string(elem);
3082+
key = ZSTR_VAL(key_zstr);
3083+
key_len = ZSTR_LEN(key_zstr);
31153084

31163085
/* Keep track of the old command pointer */
31173086
old_cmd = *ret;
@@ -3129,10 +3098,8 @@ redis_build_eval_cmd(RedisSock *redis_sock, char **ret, char *keyword,
31293098
/* Free our key, old command if we need to */
31303099
if(key_free) efree(key);
31313100

3132-
// Free our temporary arg if we created one
3133-
if(!Z_ISUNDEF(z_tmp)) {
3134-
zval_dtor(&z_tmp);
3135-
}
3101+
// Free our temporary string if we created one (or decrement refcount)
3102+
zend_string_release(key_zstr);
31363103
}
31373104
}
31383105
}
@@ -3235,12 +3202,14 @@ redis_build_script_exists_cmd(char **ret, zval *argv, int argc) {
32353202

32363203
/* Iterate our arguments */
32373204
for(i=0;i<argc;i++) {
3238-
/* Convert our argument to a string if we need to */
3239-
convert_to_string(&argv[i]);
3205+
// Get underlying string, or convert our argument to a string if we need to.
3206+
zend_string *arg_zstr = zval_get_string(&argv[i]);
32403207

32413208
// Append this script sha to our SCRIPT EXISTS command
3242-
cmd_len = redis_cmd_append_str(ret, cmd_len, Z_STRVAL(argv[i]),
3243-
Z_STRLEN(argv[i]));
3209+
cmd_len = redis_cmd_append_str(ret, cmd_len, ZSTR_VAL(arg_zstr),
3210+
ZSTR_LEN(arg_zstr));
3211+
// decrement refcount or free temporary string
3212+
zend_string_release(arg_zstr);
32443213
}
32453214

32463215
/* Success */

‎redis_commands.c

Lines changed: 25 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -665,9 +665,9 @@ int redis_zinter_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
665665
int key_free;
666666
zend_string *key_zstr;
667667

668-
key_zstr = zval_get_string(z_ele);
669-
key = ZSTR_VAL(key_zstr);
670-
key_len = ZSTR_LEN(key_zstr);
668+
key_zstr = zval_get_string(z_ele);
669+
key = ZSTR_VAL(key_zstr);
670+
key_len = ZSTR_LEN(key_zstr);
671671

672672
// Prefix key if necissary
673673
key_free = redis_key_prefix(redis_sock, &key, &key_len);
@@ -1902,15 +1902,14 @@ int redis_pfmerge_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
19021902
int redis_pfcount_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
19031903
char **cmd, int *cmd_len, short *slot, void **ctx)
19041904
{
1905-
zval *z_keys, *z_key, z_tmp;
1905+
zval *z_keys, *z_key;
19061906
HashTable *ht_keys;
19071907
HashPosition ptr;
19081908
smart_string cmdstr = {0};
19091909
int num_keys, key_free;
19101910
size_t key_len;
19111911
char *key;
19121912
short kslot=-1;
1913-
ZVAL_UNDEF(&z_tmp);
19141913

19151914
if(zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC,"z",&z_keys)==FAILURE) {
19161915
return FAILURE;
@@ -1938,30 +1937,22 @@ int redis_pfcount_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
19381937
zend_hash_move_forward_ex(ht_keys, &ptr))
19391938
{
19401939
/* Turn our value into a string if it isn't one */
1941-
if (Z_TYPE_P(z_key) != IS_STRING) {
1942-
ZVAL_DUP(&z_tmp, z_key);
1943-
convert_to_string(&z_tmp);
1944-
1945-
key = Z_STRVAL(z_tmp);
1946-
key_len = Z_STRLEN(z_tmp);
1947-
} else {
1948-
key = Z_STRVAL_P(z_key);
1949-
key_len = Z_STRLEN_P(z_key);
1950-
}
1940+
/* Aside: Could use ZEND_HASH_FOREACH_KEY_PTR instead in php7. */
1941+
zend_string *key_zstr = zval_get_string(z_key);
1942+
key = ZSTR_VAL(key_zstr);
1943+
key_len = ZSTR_LEN(key_zstr);
19511944

19521945
/* Append this key to our command */
19531946
key_free = redis_key_prefix(redis_sock, &key, &key_len);
19541947
redis_cmd_append_sstr(&cmdstr, key, key_len);
19551948

1956-
/* Protect against CROSSLOT errors */
1949+
/* Protect against CROSSSLOT errors */
19571950
if (slot) {
19581951
if (kslot == -1) {
19591952
kslot = cluster_hash_key(key, key_len);
1960-
} else if(cluster_hash_key(key,key_len)!=kslot) {
1953+
} else if (cluster_hash_key(key,key_len)!=kslot) {
19611954
if (key_free) efree(key);
1962-
if (Z_TYPE(z_tmp) != IS_UNDEF) {
1963-
zval_dtor(&z_tmp);
1964-
}
1955+
zend_string_release(key_zstr);
19651956
efree(cmdstr.c);
19661957

19671958
php_error_docref(NULL TSRMLS_CC, E_WARNING,
@@ -1972,22 +1963,13 @@ int redis_pfcount_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
19721963

19731964
/* Cleanup */
19741965
if (key_free) efree(key);
1975-
if (Z_TYPE(z_tmp) != IS_UNDEF) {
1976-
zval_dtor(&z_tmp);
1977-
}
1966+
zend_string_release(key_zstr);
19781967
}
19791968
} else {
1980-
/* Turn our key into a string if it's a different type */
1981-
if (Z_TYPE_P(z_keys) != IS_STRING) {
1982-
ZVAL_DUP(&z_tmp, z_keys);
1983-
convert_to_string(&z_tmp);
1984-
1985-
key = Z_STRVAL(z_tmp);
1986-
key_len = Z_STRLEN(z_tmp);
1987-
} else {
1988-
key = Z_STRVAL_P(z_keys);
1989-
key_len = Z_STRLEN_P(z_keys);
1990-
}
1969+
/* Get the string, or turn our key into a zend_string if it's a different type */
1970+
zend_string *z_keys_str = zval_get_string(z_keys);
1971+
key = ZSTR_VAL(z_keys_str);
1972+
key_len = ZSTR_LEN(z_keys_str);
19911973

19921974
/* Construct our whole command */
19931975
redis_cmd_init_sstr(&cmdstr, 1, "PFCOUNT", sizeof("PFCOUNT")-1);
@@ -1999,9 +1981,7 @@ int redis_pfcount_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
19991981

20001982
/* Cleanup */
20011983
if (key_free) efree(key);
2002-
if (Z_TYPE(z_tmp) != IS_UNDEF) {
2003-
zval_dtor(&z_tmp);
2004-
}
1984+
zend_string_release(z_keys_str);
20051985
}
20061986

20071987
/* Push our command and length to the caller */
@@ -2560,6 +2540,7 @@ int redis_hdel_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
25602540
char *arg;
25612541
int arg_free, i, argc = ZEND_NUM_ARGS();
25622542
size_t arg_len;
2543+
zend_string *arg_zstr;
25632544

25642545
// We need at least KEY and one member
25652546
if(argc < 2) {
@@ -2574,9 +2555,9 @@ int redis_hdel_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
25742555
}
25752556

25762557
// Get first argument (the key) as a string
2577-
convert_to_string(&z_args[0]);
2578-
arg = Z_STRVAL(z_args[0]);
2579-
arg_len = Z_STRLEN(z_args[0]);
2558+
arg_zstr = zval_get_string(&z_args[0]);
2559+
arg = ZSTR_VAL(arg_zstr);
2560+
arg_len = ZSTR_LEN(arg_zstr);
25802561

25812562
// Prefix
25822563
arg_free = redis_key_prefix(redis_sock, &arg, &arg_len);
@@ -2591,14 +2572,16 @@ int redis_hdel_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
25912572

25922573
// Iterate through the members we're removing
25932574
for(i=1;i<argc;i++) {
2594-
convert_to_string(&z_args[i]);
2595-
redis_cmd_append_sstr(&cmdstr, Z_STRVAL(z_args[i]), Z_STRLEN(z_args[i]));
2575+
zend_string *member_zstr = zval_get_string(&z_args[i]);
2576+
redis_cmd_append_sstr(&cmdstr, ZSTR_VAL(member_zstr), ZSTR_LEN(member_zstr));
2577+
zend_string_release(member_zstr);
25962578
}
25972579

25982580
// Push out values
25992581
*cmd = cmdstr.c;
26002582
*cmd_len = cmdstr.len;
26012583

2584+
zend_string_release(arg_zstr);
26022585
// Cleanup
26032586
efree(z_args);
26042587

‎redis_session.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,7 @@ PS_OPEN_FUNC(redis)
236236
sapi_module.treat_data(PARSE_STRING, estrdup(url->query), &params TSRMLS_CC);
237237

238238
if ((param = zend_hash_str_find(Z_ARRVAL(params), "weight", sizeof("weight") - 1)) != NULL) {
239-
convert_to_long_ex(param);
240-
weight = Z_LVAL_P(param);
239+
weight = zval_get_long(param);
241240
}
242241
if ((param = zend_hash_str_find(Z_ARRVAL(params), "timeout", sizeof("timeout") - 1)) != NULL) {
243242
timeout = atof(Z_STRVAL_P(param));
@@ -255,12 +254,10 @@ PS_OPEN_FUNC(redis)
255254
auth = estrndup(Z_STRVAL_P(param), Z_STRLEN_P(param));
256255
}
257256
if ((param = zend_hash_str_find(Z_ARRVAL(params), "database", sizeof("database") -1 )) != NULL) {
258-
convert_to_long_ex(param);
259-
database = Z_LVAL_P(param);
257+
database = zval_get_long(param);
260258
}
261259
if ((param = zend_hash_str_find(Z_ARRVAL(params), "retry_interval", sizeof("retry_interval") - 1)) != NULL) {
262-
convert_to_long_ex(param);
263-
retry_interval = Z_LVAL_P(param);
260+
retry_interval = zval_get_long(param);
264261
}
265262

266263
zval_ptr_dtor(&params);

0 commit comments

Comments
 (0)
0