Add RM_ServerInfoGetFieldUnsigned

rename RM_ServerInfoGetFieldNumerical RM_ServerInfoGetFieldSigned
move string2ull to util.c
fix leak in RM_GetServerInfo when duplicate info fields exist

7 files changed, 66 insertions, 28 deletions

diff --git a/src/module.c b/src/module.c
index fea66f293..2644883c9 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5497,7 +5497,8 @@ RedisModuleServerInfoData *RM_GetServerInfo(RedisModuleCtx *ctx, const char *sec
         unsigned char *key = (unsigned char*)line;
         size_t keylen = (intptr_t)sep-(intptr_t)line;
         sds val = sdsnewlen(sep+1,sdslen(line)-((intptr_t)sep-(intptr_t)line)-1);
-        raxTryInsert(d->rax,key,keylen,val,NULL);
+        if (!raxTryInsert(d->rax,key,keylen,val,NULL))
+            sdsfree(val);
     }
     sdsfree(info);
     sdsfreesplitres(lines,totlines);
@@ -5542,9 +5543,9 @@ const char *RM_ServerInfoGetFieldC(RedisModuleServerInfoData *data, const char*
 }
 
 /* Get the value of a field from data collected with RM_GetServerInfo(). If the
- * field is not found, or is not numerical, return value will be 0, and the
- * optional out_err argument will be set to REDISMODULE_ERR. */
-long long RM_ServerInfoGetFieldNumerical(RedisModuleServerInfoData *data, const char* field, int *out_err) {
+ * field is not found, or is not numerical or out of range, return value will be
+ * 0, and the optional out_err argument will be set to REDISMODULE_ERR. */
+long long RM_ServerInfoGetFieldSigned(RedisModuleServerInfoData *data, const char* field, int *out_err) {
     long long ll;
     sds val = raxFind(data->rax, (unsigned char *)field, strlen(field));
     if (val == raxNotFound) {
@@ -5560,6 +5561,24 @@ long long RM_ServerInfoGetFieldNumerical(RedisModuleServerInfoData *data, const
 }
 
 /* Get the value of a field from data collected with RM_GetServerInfo(). If the
+ * field is not found, or is not numerical or out of range, return value will be
+ * 0, and the optional out_err argument will be set to REDISMODULE_ERR. */
+unsigned long long RM_ServerInfoGetFieldUnsigned(RedisModuleServerInfoData *data, const char* field, int *out_err) {
+    unsigned long long ll;
+    sds val = raxFind(data->rax, (unsigned char *)field, strlen(field));
+    if (val == raxNotFound) {
+        if (out_err) *out_err = REDISMODULE_ERR;
+        return 0;
+    }
+    if (!string2ull(val,&ll)) {
+        if (out_err) *out_err = REDISMODULE_ERR;
+        return 0;
+    }
+    if (out_err) *out_err = REDISMODULE_OK;
+    return ll;
+}
+
+/* Get the value of a field from data collected with RM_GetServerInfo(). If the
  * field is not found, or is not a double, return value will be 0, and the
  * optional out_err argument will be set to REDISMODULE_ERR. */
 double RM_ServerInfoGetFieldDouble(RedisModuleServerInfoData *data, const char* field, int *out_err) {
@@ -6868,7 +6887,8 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(FreeServerInfo);
     REGISTER_API(ServerInfoGetField);
     REGISTER_API(ServerInfoGetFieldC);
-    REGISTER_API(ServerInfoGetFieldNumerical);
+    REGISTER_API(ServerInfoGetFieldSigned);
+    REGISTER_API(ServerInfoGetFieldUnsigned);
     REGISTER_API(ServerInfoGetFieldDouble);
     REGISTER_API(GetClientInfoById);
     REGISTER_API(SubscribeToServerEvent);
diff --git a/src/redismodule.h b/src/redismodule.h
index 0c289848f..fd6dc77ce 100644
--- a/src/redismodule.h
+++ b/src/redismodule.h
@@ -507,7 +507,8 @@ RedisModuleServerInfoData *REDISMODULE_API_FUNC(RedisModule_GetServerInfo)(Redis
 void REDISMODULE_API_FUNC(RedisModule_FreeServerInfo)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_ServerInfoGetField)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field);
 const char *REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldC)(RedisModuleServerInfoData *data, const char* field);
-long long REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldNumerical)(RedisModuleServerInfoData *data, const char* field, int *out_err);
+long long REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldSigned)(RedisModuleServerInfoData *data, const char* field, int *out_err);
+unsigned long long REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldUnsigned)(RedisModuleServerInfoData *data, const char* field, int *out_err);
 double REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldDouble)(RedisModuleServerInfoData *data, const char* field, int *out_err);
 int REDISMODULE_API_FUNC(RedisModule_SubscribeToServerEvent)(RedisModuleCtx *ctx, RedisModuleEvent event, RedisModuleEventCallback callback);
 RedisModuleBlockedClient *REDISMODULE_API_FUNC(RedisModule_BlockClientOnKeys)(RedisModuleCtx *ctx, RedisModuleCmdFunc reply_callback, RedisModuleCmdFunc timeout_callback, void (*free_privdata)(RedisModuleCtx*,void*), long long timeout_ms, RedisModuleString **keys, int numkeys, void *privdata);
@@ -715,7 +716,8 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int
     REDISMODULE_GET_API(FreeServerInfo);
     REDISMODULE_GET_API(ServerInfoGetField);
     REDISMODULE_GET_API(ServerInfoGetFieldC);
-    REDISMODULE_GET_API(ServerInfoGetFieldNumerical);
+    REDISMODULE_GET_API(ServerInfoGetFieldSigned);
+    REDISMODULE_GET_API(ServerInfoGetFieldUnsigned);
     REDISMODULE_GET_API(ServerInfoGetFieldDouble);
     REDISMODULE_GET_API(GetClientInfoById);
     REDISMODULE_GET_API(SubscribeToServerEvent);
diff --git a/src/t_stream.c b/src/t_stream.c
index ea9a620f1..e6694f0b7 100644
--- a/src/t_stream.c
+++ b/src/t_stream.c
@@ -1070,26 +1070,6 @@ robj *streamTypeLookupWriteOrCreate(client *c, robj *key) {
     return o;
 }
 
-/* Helper function to convert a string to an unsigned long long value.
- * The function attempts to use the faster string2ll() function inside
- * Redis: if it fails, strtoull() is used instead. The function returns
- * 1 if the conversion happened successfully or 0 if the number is
- * invalid or out of range. */
-int string2ull(const char *s, unsigned long long *value) {
-    long long ll;
-    if (string2ll(s,strlen(s),&ll)) {
-        if (ll < 0) return 0; /* Negative values are out of range. */
-        *value = ll;
-        return 1;
-    }
-    errno = 0;
-    char *endptr = NULL;
-    *value = strtoull(s,&endptr,10);
-    if (errno == EINVAL || errno == ERANGE || !(*s != '\0' && *endptr == '\0'))
-        return 0; /* strtoull() failed. */
-    return 1; /* Conversion done! */
-}
-
 /* Parse a stream ID in the format given by clients to Redis, that is
  * <ms>-<seq>, and converts it into a streamID structure. If
  * the specified ID is invalid C_ERR is returned and an error is reported
diff --git a/src/util.c b/src/util.c
index 6e9dc2117..f0f1a4ed3 100644
--- a/src/util.c
+++ b/src/util.c
@@ -423,6 +423,26 @@ int string2ll(const char *s, size_t slen, long long *value) {
     return 1;
 }
 
+/* Helper function to convert a string to an unsigned long long value.
+ * The function attempts to use the faster string2ll() function inside
+ * Redis: if it fails, strtoull() is used instead. The function returns
+ * 1 if the conversion happened successfully or 0 if the number is
+ * invalid or out of range. */
+int string2ull(const char *s, unsigned long long *value) {
+    long long ll;
+    if (string2ll(s,strlen(s),&ll)) {
+        if (ll < 0) return 0; /* Negative values are out of range. */
+        *value = ll;
+        return 1;
+    }
+    errno = 0;
+    char *endptr = NULL;
+    *value = strtoull(s,&endptr,10);
+    if (errno == EINVAL || errno == ERANGE || !(*s != '\0' && *endptr == '\0'))
+        return 0; /* strtoull() failed. */
+    return 1; /* Conversion done! */
+}
+
 /* Convert a string into a long. Returns 1 if the string could be parsed into a
  * (non-overflowing) long, 0 otherwise. The value will be set to the parsed
  * value when appropriate. */
diff --git a/src/util.h b/src/util.h
index ab1d71f6c..7e162686e 100644
--- a/src/util.h
+++ b/src/util.h
@@ -46,6 +46,7 @@ uint32_t digits10(uint64_t v);
 uint32_t sdigits10(int64_t v);
 int ll2string(char *s, size_t len, long long value);
 int string2ll(const char *s, size_t slen, long long *value);
+int string2ull(const char *s, unsigned long long *value);
 int string2l(const char *s, size_t slen, long *value);
 int string2ld(const char *s, size_t slen, long double *dp);
 int string2d(const char *s, size_t slen, double *dp);
diff --git a/tests/modules/infotest.c b/tests/modules/infotest.c
index a21fefffc..4cb77ee87 100644
--- a/tests/modules/infotest.c
+++ b/tests/modules/infotest.c
@@ -5,6 +5,7 @@
 void InfoFunc(RedisModuleInfoCtx *ctx, int for_crash_report) {
     RedisModule_InfoAddSection(ctx, "");
     RedisModule_InfoAddFieldLongLong(ctx, "global", -2);
+    RedisModule_InfoAddFieldULongLong(ctx, "uglobal", (unsigned long long)-2);
 
     RedisModule_InfoAddSection(ctx, "Spanish");
     RedisModule_InfoAddFieldCString(ctx, "uno", "one");
@@ -41,7 +42,11 @@ int info_get(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, char field
     field = RedisModule_StringPtrLen(argv[2], NULL);
     RedisModuleServerInfoData *info = RedisModule_GetServerInfo(ctx, section);
     if (field_type=='i') {
-        long long ll = RedisModule_ServerInfoGetFieldNumerical(info, field, &err);
+        long long ll = RedisModule_ServerInfoGetFieldSigned(info, field, &err);
+        if (err==REDISMODULE_OK)
+            RedisModule_ReplyWithLongLong(ctx, ll);
+    } else if (field_type=='u') {
+        unsigned long long ll = (unsigned long long)RedisModule_ServerInfoGetFieldUnsigned(info, field, &err);
         if (err==REDISMODULE_OK)
             RedisModule_ReplyWithLongLong(ctx, ll);
     } else if (field_type=='d') {
@@ -78,6 +83,10 @@ int info_geti(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
     return info_get(ctx, argv, argc, 'i');
 }
 
+int info_getu(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+    return info_get(ctx, argv, argc, 'u');
+}
+
 int info_getd(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
     return info_get(ctx, argv, argc, 'd');
 }
@@ -96,6 +105,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
         return REDISMODULE_ERR;
     if (RedisModule_CreateCommand(ctx,"info.geti", info_geti,"",0,0,0) == REDISMODULE_ERR)
         return REDISMODULE_ERR;
+    if (RedisModule_CreateCommand(ctx,"info.getu", info_getu,"",0,0,0) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
     if (RedisModule_CreateCommand(ctx,"info.getd", info_getd,"",0,0,0) == REDISMODULE_ERR)
         return REDISMODULE_ERR;
 
diff --git a/tests/unit/moduleapi/infotest.tcl b/tests/unit/moduleapi/infotest.tcl
index 225798dd5..80a28656c 100644
--- a/tests/unit/moduleapi/infotest.tcl
+++ b/tests/unit/moduleapi/infotest.tcl
@@ -17,6 +17,10 @@ start_server {tags {"modules"}} {
         assert_equal [r info.geti stats expired_keys] 0
         assert_equal [r info.getd stats expired_stale_perc] 0
 
+        # check signed and unsigned
+        assert_equal [r info.geti infotest infotest_global] -2
+        assert_equal [r info.getu infotest infotest_uglobal] -2
+
         # the above are always 0, try module info that is non-zero
         assert_equal [r info.geti infotest_italian infotest_due] 2
         set tre [r info.getd infotest_italian infotest_tre]