Skip to content

Commit

Permalink
32bit xclaim fix (#1444)
Browse files Browse the repository at this point in the history
This should fix the XCLAIM issue on 32-bit PHP installs.

This change will allow the user to pass the XCLAIM TIME option pretty much any way they want (string, long, or float) and it should work.  Note that in 32-bit PHP they will only be able to pass exact values <= 2^53 as PHP will use a double precision floating point for integer overflows.
  • Loading branch information
michael-grunder committed Nov 6, 2018
1 parent b509391 commit 18dc2aa
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 15 deletions.
2 changes: 2 additions & 0 deletions common.h
Original file line number Diff line number Diff line change
Expand Up @@ -461,8 +461,10 @@ typedef size_t strlen_t;

#ifdef PHP_WIN32
#define PHP_REDIS_API __declspec(dllexport)
#define phpredis_atoi64(p) _atoi64((p))
#else
#define PHP_REDIS_API
#define phpredis_atoi64(p) atoll((p))
#endif

/* reply types */
Expand Down
16 changes: 11 additions & 5 deletions library.c
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,15 @@ int redis_cmd_append_sstr_long(smart_string *str, long append) {
return redis_cmd_append_sstr(str, long_buf, long_len);
}

/*
* Append a 64-bit integer to our command
*/
int redis_cmd_append_sstr_i64(smart_string *str, int64_t append) {
char nbuf[64];
int len = snprintf(nbuf, sizeof(nbuf), "%lld", append);
return redis_cmd_append_sstr(str, nbuf, len);
}

/*
* Append a double to a smart string command
*/
Expand Down Expand Up @@ -1080,11 +1089,8 @@ PHP_REDIS_API void redis_long_response(INTERNAL_FUNCTION_PARAMETERS,
}

if(response[0] == ':') {
#ifdef PHP_WIN32
__int64 ret = _atoi64(response + 1);
#else
long long ret = atoll(response + 1);
#endif
int64_t ret = phpredis_atoi64(response + 1);

if (IS_ATOMIC(redis_sock)) {
if(ret > LONG_MAX) { /* overflow */
RETVAL_STRINGL(response + 1, response_len - 1);
Expand Down
1 change: 1 addition & 0 deletions library.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ int redis_cmd_init_sstr(smart_string *str, int num_args, char *keyword, int keyw
int redis_cmd_append_sstr(smart_string *str, char *append, int append_len);
int redis_cmd_append_sstr_int(smart_string *str, int append);
int redis_cmd_append_sstr_long(smart_string *str, long append);
int redis_cmd_append_sstr_i64(smart_string *str, int64_t append);
int redis_cmd_append_sstr_dbl(smart_string *str, double value);
int redis_cmd_append_sstr_zval(smart_string *str, zval *z, RedisSock *redis_sock TSRMLS_DC);
int redis_cmd_append_sstr_key(smart_string *str, char *key, strlen_t len, RedisSock *redis_sock, short *slot);
Expand Down
59 changes: 49 additions & 10 deletions redis_commands.c
Original file line number Diff line number Diff line change
Expand Up @@ -3525,13 +3525,56 @@ int redis_xack_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
typedef struct xclaimOptions {
struct {
char *type;
zend_long time;
int64_t time;
} idle;
zend_long retrycount;
int force;
int justid;
} xclaimOptions;

/* Attempt to extract an int64_t from the provided zval */
static int zval_get_i64(zval *zv, int64_t *retval) {
if (Z_TYPE_P(zv) == IS_LONG) {
*retval = (int64_t)Z_LVAL_P(zv);
return SUCCESS;
} else if (Z_TYPE_P(zv) == IS_DOUBLE) {
*retval = (int64_t)Z_DVAL_P(zv);
return SUCCESS;
} else if (Z_TYPE_P(zv) == IS_STRING) {
zend_long lval;
double dval;

switch (is_numeric_string(Z_STRVAL_P(zv), Z_STRLEN_P(zv), &lval, &dval, 1)) {
case IS_LONG:
*retval = (int64_t)lval;
return SUCCESS;
case IS_DOUBLE:
*retval = (int64_t)dval;
return SUCCESS;
}
}

/* If we make it here we have failed */
return FAILURE;
}

/* Helper function to get an integer XCLAIM argument. This can overflow a
* 32-bit PHP long so we have to extract it as an int64_t. If the value is
* not a valid number or negative, we'll inform the user of the problem and
* that the argument is being ignored. */
static int64_t get_xclaim_i64_arg(const char *key, zval *zv TSRMLS_DC) {
int64_t retval = -1;

/* Extract an i64, and if we can't let the user know there is an issue. */
if (zval_get_i64(zv, &retval) == FAILURE || retval < 0) {
php_error_docref(NULL TSRMLS_CC, E_WARNING,
"Invalid XCLAIM option '%s' will be ignored", key);
}

/* Success */
return retval;
}

/* Helper to extract XCLAIM options */
static void get_xclaim_options(zval *z_arr, xclaimOptions *opt TSRMLS_DC) {
HashTable *ht;
Expand All @@ -3556,23 +3599,19 @@ static void get_xclaim_options(zval *z_arr, xclaimOptions *opt TSRMLS_DC) {
ht = Z_ARRVAL_P(z_arr);
ZEND_HASH_FOREACH_KEY_VAL(ht, idx, zkey, zv) {
if (zkey) {
/* Every key => value xclaim option requires a long and Redis
* treats -1 as not being passed so skip negative values too. */
if (Z_TYPE_P(zv) != IS_LONG || Z_LVAL_P(zv) < 0)
continue;

kval = ZSTR_VAL(zkey);
klen = ZSTR_LEN(zkey);

if (klen == 4) {
if (!strncasecmp(kval, "TIME", 4)) {
opt->idle.type = "TIME";
opt->idle.time = Z_LVAL_P(zv);
opt->idle.time = get_xclaim_i64_arg("TIME", zv TSRMLS_CC);
} else if (!strncasecmp(kval, "IDLE", 4)) {
opt->idle.type = "IDLE";
opt->idle.time = Z_LVAL_P(zv);
opt->idle.time = get_xclaim_i64_arg("IDLE", zv TSRMLS_CC);
}
} else if (klen == 10 && !strncasecmp(kval, "RETRYCOUNT", 10)) {
opt->retrycount = Z_LVAL_P(zv);
opt->retrycount = zval_get_long(zv);
}
} else {
if (Z_TYPE_P(zv) == IS_STRING) {
Expand Down Expand Up @@ -3609,7 +3648,7 @@ static void append_xclaim_options(smart_string *cmd, xclaimOptions *opt) {
/* IDLE/TIME long */
if (opt->idle.type != NULL && opt->idle.time != -1) {
redis_cmd_append_sstr(cmd, opt->idle.type, strlen(opt->idle.type));
redis_cmd_append_sstr_long(cmd, opt->idle.time);
redis_cmd_append_sstr_i64(cmd, opt->idle.time);
}

/* RETRYCOUNT */
Expand Down

0 comments on commit 18dc2aa

Please sign in to comment.