From 4d580438b0ce8b3e02213a01f892a34f36cde144 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Sun, 3 Nov 2019 15:02:25 +0200
Subject: [PATCH 1/3] Add module api for looking into INFO fields

- Add RM_GetServerInfo and friends
- Add auto memory for new opaque struct
- Add tests for new APIs

other minor fixes:
- add const in various char pointers
- requested_section in modulesCollectInfo was actually not sds but char*
- extract new string2d out of getDoubleFromObject for code reuse

Add module API for
---
 src/module.c                      | 112 +++++++++++++++++++++++++++++-
 src/object.c                      |  10 +--
 src/redismodule.h                 |  11 +++
 src/server.c                      |   2 +-
 src/server.h                      |   4 +-
 src/util.c                        |  21 ++++++
 src/util.h                        |   1 +
 tests/modules/infotest.c          |  52 ++++++++++++++
 tests/unit/moduleapi/infotest.tcl |  21 ++++++
 9 files changed, 220 insertions(+), 14 deletions(-)

diff --git a/src/module.c b/src/module.c
index f9f654b42..ce85c479d 100644
--- a/src/module.c
+++ b/src/module.c
@@ -41,7 +41,7 @@
 
 typedef struct RedisModuleInfoCtx {
     struct RedisModule *module;
-    sds requested_section;
+    const char *requested_section;
     sds info;           /* info string we collected so far */
     int sections;       /* number of sections we collected so far */
     int in_section;     /* indication if we're in an active section or not */
@@ -93,6 +93,7 @@ struct AutoMemEntry {
 #define REDISMODULE_AM_REPLY 2
 #define REDISMODULE_AM_FREED 3 /* Explicitly freed by user already. */
 #define REDISMODULE_AM_DICT 4
+#define REDISMODULE_AM_INFO 5
 
 /* The pool allocator block. Redis Modules can allocate memory via this special
  * allocator that will automatically release it all once the callback returns.
@@ -322,6 +323,10 @@ static struct RedisModuleForkInfo {
     void* done_handler_user_data;
 } moduleForkInfo = {0};
 
+typedef struct RedisModuleServerInfoData {
+    rax *rax;                       /* parsed info data. */
+} RedisModuleServerInfoData;
+
 /* Flags for moduleCreateArgvFromUserFormat(). */
 #define REDISMODULE_ARGV_REPLICATE (1<<0)
 #define REDISMODULE_ARGV_NO_AOF (1<<1)
@@ -361,6 +366,7 @@ void moduleReplicateMultiIfNeeded(RedisModuleCtx *ctx);
 void RM_ZsetRangeStop(RedisModuleKey *kp);
 static void zsetKeyReset(RedisModuleKey *key);
 void RM_FreeDict(RedisModuleCtx *ctx, RedisModuleDict *d);
+void RM_FreeServerInfo(RedisModuleCtx *ctx, RedisModuleServerInfoData *data);
 
 /* --------------------------------------------------------------------------
  * Heap allocation raw functions
@@ -946,6 +952,7 @@ void autoMemoryCollect(RedisModuleCtx *ctx) {
         case REDISMODULE_AM_REPLY: RM_FreeCallReply(ptr); break;
         case REDISMODULE_AM_KEY: RM_CloseKey(ptr); break;
         case REDISMODULE_AM_DICT: RM_FreeDict(NULL,ptr); break;
+        case REDISMODULE_AM_INFO: RM_FreeServerInfo(NULL,ptr); break;
         }
     }
     ctx->flags |= REDISMODULE_CTX_AUTO_MEMORY;
@@ -5449,7 +5456,7 @@ int RM_RegisterInfoFunc(RedisModuleCtx *ctx, RedisModuleInfoFunc cb) {
     return REDISMODULE_OK;
 }
 
-sds modulesCollectInfo(sds info, sds section, int for_crash_report, int sections) {
+sds modulesCollectInfo(sds info, const char *section, int for_crash_report, int sections) {
     dictIterator *di = dictGetIterator(modules);
     dictEntry *de;
 
@@ -5469,6 +5476,102 @@ sds modulesCollectInfo(sds info, sds section, int for_crash_report, int sections
     return info;
 }
 
+/* Get information about the server similar to the one that returns from the
+ * INFO command. This function takes an optional 'section' argument that may
+ * be NULL. The return value holds the output and can be used with
+ * RedisModule_ServerInfoGetField and alike to get the individual fields.
+ * When done, it needs to be freed with RedisModule_FreeServerInfo or with the
+ * automatic memory management mechanism if enabled. */
+RedisModuleServerInfoData *RM_GetServerInfo(RedisModuleCtx *ctx, const char *section) {
+    struct RedisModuleServerInfoData *d = zmalloc(sizeof(*d));
+    d->rax = raxNew();
+    if (ctx != NULL) autoMemoryAdd(ctx,REDISMODULE_AM_INFO,d);
+    sds info = genRedisInfoString(section);
+    int totlines, i;
+    sds *lines = sdssplitlen(info, sdslen(info), "\r\n", 2, &totlines);
+    for(i=0; i<totlines; i++) {
+        sds line = lines[i];
+        if (line[0]=='#') continue;
+        char *sep = strchr(line, ':');
+        if (!sep) continue;
+        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);
+    }
+    sdsfree(info);
+    sdsfreesplitres(lines,totlines);
+    return d;
+}
+
+/* Free data created with RM_GetServerInfo(). You need to pass the
+ * context pointer 'ctx' only if the dictionary was created using the
+ * context instead of passing NULL. */
+void RM_FreeServerInfo(RedisModuleCtx *ctx, RedisModuleServerInfoData *data) {
+    if (ctx != NULL) autoMemoryFreed(ctx,REDISMODULE_AM_INFO,data);
+    raxIterator ri;
+    raxStart(&ri,data->rax);
+    while(1) {
+        raxSeek(&ri,"^",NULL,0);
+        if (!raxNext(&ri)) break;
+        raxRemove(data->rax,(unsigned char*)ri.key,ri.key_len,NULL);
+        sdsfree(ri.data);
+    }
+    raxStop(&ri);
+    raxFree(data->rax);
+    zfree(data);
+}
+
+/* Get the value of a field from data collected with RM_GetServerInfo(). You
+ * need to pass the context pointer 'ctx' only if you want to use auto memory
+ * mechanism to release the returned string. Return value will be NULL if the
+ * field was not found. */
+RedisModuleString *RM_ServerInfoGetField(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field) {
+    sds val = raxFind(data->rax, (unsigned char *)field, strlen(field));
+    if (val == raxNotFound) return NULL;
+    RedisModuleString *o = createStringObject(val,sdslen(val));
+    if (ctx != NULL) autoMemoryAdd(ctx,REDISMODULE_AM_STRING,o);
+    return o;
+}
+
+/* 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(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field, int *out_err) {
+    UNUSED(ctx);
+    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 (!string2ll(val,sdslen(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(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field, int *out_err) {
+    UNUSED(ctx);
+    double dbl;
+    sds val = raxFind(data->rax, (unsigned char *)field, strlen(field));
+    if (val == raxNotFound) {
+        if (out_err) *out_err = REDISMODULE_ERR;
+        return 0;
+    }
+    if (!string2d(val,sdslen(val),&dbl)) {
+        if (out_err) *out_err = REDISMODULE_ERR;
+        return 0;
+    }
+    if (out_err) *out_err = REDISMODULE_OK;
+    return dbl;
+}
+
 /* --------------------------------------------------------------------------
  * Modules utility APIs
  * -------------------------------------------------------------------------- */
@@ -6756,6 +6859,11 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(InfoAddFieldDouble);
     REGISTER_API(InfoAddFieldLongLong);
     REGISTER_API(InfoAddFieldULongLong);
+    REGISTER_API(GetServerInfo);
+    REGISTER_API(FreeServerInfo);
+    REGISTER_API(ServerInfoGetField);
+    REGISTER_API(ServerInfoGetFieldNumerical);
+    REGISTER_API(ServerInfoGetFieldDouble);
     REGISTER_API(GetClientInfoById);
     REGISTER_API(SubscribeToServerEvent);
     REGISTER_API(BlockClientOnKeys);
diff --git a/src/object.c b/src/object.c
index 70022f897..7ce79bbbb 100644
--- a/src/object.c
+++ b/src/object.c
@@ -606,21 +606,13 @@ size_t stringObjectLen(robj *o) {
 
 int getDoubleFromObject(const robj *o, double *target) {
     double value;
-    char *eptr;
 
     if (o == NULL) {
         value = 0;
     } else {
         serverAssertWithInfo(NULL,o,o->type == OBJ_STRING);
         if (sdsEncodedObject(o)) {
-            errno = 0;
-            value = strtod(o->ptr, &eptr);
-            if (sdslen(o->ptr) == 0 ||
-                isspace(((const char*)o->ptr)[0]) ||
-                (size_t)(eptr-(char*)o->ptr) != sdslen(o->ptr) ||
-                (errno == ERANGE &&
-                    (value == HUGE_VAL || value == -HUGE_VAL || value == 0)) ||
-                isnan(value))
+            if (!string2d(o->ptr, sdslen(o->ptr), &value))
                 return C_ERR;
         } else if (o->encoding == OBJ_ENCODING_INT) {
             value = (long)o->ptr;
diff --git a/src/redismodule.h b/src/redismodule.h
index ea0d6a139..d923a1b8a 100644
--- a/src/redismodule.h
+++ b/src/redismodule.h
@@ -318,6 +318,7 @@ typedef struct RedisModuleDictIter RedisModuleDictIter;
 typedef struct RedisModuleCommandFilterCtx RedisModuleCommandFilterCtx;
 typedef struct RedisModuleCommandFilter RedisModuleCommandFilter;
 typedef struct RedisModuleInfoCtx RedisModuleInfoCtx;
+typedef struct RedisModuleServerInfoData RedisModuleServerInfoData;
 
 typedef int (*RedisModuleCmdFunc)(RedisModuleCtx *ctx, RedisModuleString **argv, int argc);
 typedef void (*RedisModuleDisconnectFunc)(RedisModuleCtx *ctx, RedisModuleBlockedClient *bc);
@@ -502,6 +503,11 @@ int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldCString)(RedisModuleInfoCtx *ct
 int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldDouble)(RedisModuleInfoCtx *ctx, char *field, double value);
 int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldLongLong)(RedisModuleInfoCtx *ctx, char *field, long long value);
 int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldULongLong)(RedisModuleInfoCtx *ctx, char *field, unsigned long long value);
+RedisModuleServerInfoData *REDISMODULE_API_FUNC(RedisModule_GetServerInfo)(RedisModuleCtx *ctx, const char *section);
+void REDISMODULE_API_FUNC(RedisModule_FreeServerInfo)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data);
+RedisModuleString *REDISMODULE_API_FUNC(RedisModule_ServerInfoGetField)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field);
+long long REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldNumerical)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field, int *out_err);
+double REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldDouble)(RedisModuleCtx *ctx, 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);
 void REDISMODULE_API_FUNC(RedisModule_SignalKeyAsReady)(RedisModuleCtx *ctx, RedisModuleString *key);
@@ -704,6 +710,11 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int
     REDISMODULE_GET_API(InfoAddFieldDouble);
     REDISMODULE_GET_API(InfoAddFieldLongLong);
     REDISMODULE_GET_API(InfoAddFieldULongLong);
+    REDISMODULE_GET_API(GetServerInfo);
+    REDISMODULE_GET_API(FreeServerInfo);
+    REDISMODULE_GET_API(ServerInfoGetField);
+    REDISMODULE_GET_API(ServerInfoGetFieldNumerical);
+    REDISMODULE_GET_API(ServerInfoGetFieldDouble);
     REDISMODULE_GET_API(GetClientInfoById);
     REDISMODULE_GET_API(SubscribeToServerEvent);
     REDISMODULE_GET_API(BlockClientOnKeys);
diff --git a/src/server.c b/src/server.c
index 8f165113d..a11cb538d 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3909,7 +3909,7 @@ void bytesToHuman(char *s, unsigned long long n) {
 /* Create the string returned by the INFO command. This is decoupled
  * by the INFO command itself as we need to report the same information
  * on memory corruption problems. */
-sds genRedisInfoString(char *section) {
+sds genRedisInfoString(const char *section) {
     sds info = sdsempty();
     time_t uptime = server.unixtime-server.stat_starttime;
     int j;
diff --git a/src/server.h b/src/server.h
index f724f7d64..9932bafa8 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1600,7 +1600,7 @@ void ModuleForkDoneHandler(int exitcode, int bysignal);
 int TerminateModuleForkChild(int child_pid, int wait);
 ssize_t rdbSaveModulesAux(rio *rdb, int when);
 int moduleAllDatatypesHandleErrors();
-sds modulesCollectInfo(sds info, sds section, int for_crash_report, int sections);
+sds modulesCollectInfo(sds info, const char *section, int for_crash_report, int sections);
 void moduleFireServerEvent(uint64_t eid, int subid, void *data);
 int moduleTryServeClientBlockedOnKey(client *c, robj *key);
 void moduleUnblockClient(client *c);
@@ -2409,7 +2409,7 @@ void _serverPanic(const char *file, int line, const char *msg, ...);
 void bugReportStart(void);
 void serverLogObjectDebugInfo(const robj *o);
 void sigsegvHandler(int sig, siginfo_t *info, void *secret);
-sds genRedisInfoString(char *section);
+sds genRedisInfoString(const char *section);
 sds genModulesInfoString(sds info);
 void enableWatchdog(int period);
 void disableWatchdog(void);
diff --git a/src/util.c b/src/util.c
index 783bcf83b..6e9dc2117 100644
--- a/src/util.c
+++ b/src/util.c
@@ -468,6 +468,27 @@ int string2ld(const char *s, size_t slen, long double *dp) {
     return 1;
 }
 
+/* Convert a string into a double. Returns 1 if the string could be parsed
+ * into a (non-overflowing) double, 0 otherwise. The value will be set to
+ * the parsed value when appropriate.
+ *
+ * Note that this function demands that the string strictly represents
+ * a double: no spaces or other characters before or after the string
+ * representing the number are accepted. */
+int string2d(const char *s, size_t slen, double *dp) {
+    errno = 0;
+    char *eptr;
+    *dp = strtod(s, &eptr);
+    if (slen == 0 ||
+        isspace(((const char*)s)[0]) ||
+        (size_t)(eptr-(char*)s) != slen ||
+        (errno == ERANGE &&
+            (*dp == HUGE_VAL || *dp == -HUGE_VAL || *dp == 0)) ||
+        isnan(*dp))
+        return 0;
+    return 1;
+}
+
 /* Convert a double to a string representation. Returns the number of bytes
  * required. The representation should always be parsable by strtod(3).
  * This function does not support human-friendly formatting like ld2string
diff --git a/src/util.h b/src/util.h
index b6c01aa59..ab1d71f6c 100644
--- a/src/util.h
+++ b/src/util.h
@@ -48,6 +48,7 @@ int ll2string(char *s, size_t len, long long value);
 int string2ll(const char *s, size_t slen, 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);
 int d2string(char *buf, size_t len, double value);
 int ld2string(char *buf, size_t len, long double value, int humanfriendly);
 sds getAbsolutePath(char *filename);
diff --git a/tests/modules/infotest.c b/tests/modules/infotest.c
index d28410932..460ebb9c0 100644
--- a/tests/modules/infotest.c
+++ b/tests/modules/infotest.c
@@ -29,6 +29,51 @@ void InfoFunc(RedisModuleInfoCtx *ctx, int for_crash_report) {
 
 }
 
+int info_get(RedisModuleCtx *ctx, RedisModuleString **argv, int argc, char field_type)
+{
+    if (argc != 3 && argc != 4) {
+        RedisModule_WrongArity(ctx);
+        return REDISMODULE_OK;
+    }
+    int err = REDISMODULE_OK;
+    const char *section, *field;
+    section = RedisModule_StringPtrLen(argv[1], NULL);
+    field = RedisModule_StringPtrLen(argv[2], NULL);
+    RedisModuleServerInfoData *info = RedisModule_GetServerInfo(ctx, section);
+    if (field_type=='i') {
+        long long ll = RedisModule_ServerInfoGetFieldNumerical(ctx, info, field, &err);
+        if (err==REDISMODULE_OK)
+            RedisModule_ReplyWithLongLong(ctx, ll);
+    } else if (field_type=='d') {
+        double d = RedisModule_ServerInfoGetFieldDouble(ctx, info, field, &err);
+        if (err==REDISMODULE_OK)
+            RedisModule_ReplyWithDouble(ctx, d);
+    } else {
+        RedisModuleString *str = RedisModule_ServerInfoGetField(ctx, info, field);
+        if (str) {
+            RedisModule_ReplyWithString(ctx, str);
+            RedisModule_FreeString(ctx, str);
+        } else
+            err=REDISMODULE_ERR;
+    }
+    if (err!=REDISMODULE_OK)
+        RedisModule_ReplyWithError(ctx, "not found");
+    RedisModule_FreeServerInfo(ctx, info);
+    return REDISMODULE_OK;
+}
+
+int info_gets(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+    return info_get(ctx, argv, argc, 's');
+}
+
+int info_geti(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+    return info_get(ctx, argv, argc, 'i');
+}
+
+int info_getd(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+    return info_get(ctx, argv, argc, 'd');
+}
+
 int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
     REDISMODULE_NOT_USED(argv);
     REDISMODULE_NOT_USED(argc);
@@ -37,5 +82,12 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 
     if (RedisModule_RegisterInfoFunc(ctx, InfoFunc) == REDISMODULE_ERR) return REDISMODULE_ERR;
 
+    if (RedisModule_CreateCommand(ctx,"info.gets", info_gets,"",0,0,0) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
+    if (RedisModule_CreateCommand(ctx,"info.geti", info_geti,"",0,0,0) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
+    if (RedisModule_CreateCommand(ctx,"info.getd", info_getd,"",0,0,0) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
+
     return REDISMODULE_OK;
 }
diff --git a/tests/unit/moduleapi/infotest.tcl b/tests/unit/moduleapi/infotest.tcl
index 659ee79d7..a42681345 100644
--- a/tests/unit/moduleapi/infotest.tcl
+++ b/tests/unit/moduleapi/infotest.tcl
@@ -10,6 +10,27 @@ proc field {info property} {
 start_server {tags {"modules"}} {
     r module load $testmodule log-key 0
 
+    test {module reading info} {
+        # check string, integer and float fields
+        assert_equal [r info.gets replication role] "master"
+        assert_equal [r info.geti stats expired_keys] 0
+        assert_equal [r info.getd stats expired_stale_perc] 0
+
+        # 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]
+        assert {$tre > 3.2 && $tre < 3.4 }
+
+        # search using the wrong section
+        catch { [r info.gets badname redis_version] } e
+        assert_match {*not found*} $e
+
+        # check that section filter works
+        assert { [string match "*usec_per_call*" [r info.gets all cmdstat_info.gets] ] }
+        catch { [r info.gets default cmdstat_info.gets] ] } e
+        assert_match {*not found*} $e
+    }
+
     test {module info all} {
         set info [r info all]
         # info all does not contain modules

From deebed23e16c2cab9e0362e94bca8545d6e33596 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Mon, 4 Nov 2019 07:57:52 +0200
Subject: [PATCH 2/3] Adding RM_ServerInfoGetFieldC

---
 src/module.c                      | 14 ++++++++++----
 src/redismodule.h                 |  6 ++++--
 tests/modules/infotest.c          | 14 ++++++++++++--
 tests/unit/moduleapi/infotest.tcl |  1 +
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/module.c b/src/module.c
index ce85c479d..fea66f293 100644
--- a/src/module.c
+++ b/src/module.c
@@ -5534,11 +5534,17 @@ RedisModuleString *RM_ServerInfoGetField(RedisModuleCtx *ctx, RedisModuleServerI
     return o;
 }
 
+/* Similar to RM_ServerInfoGetField, but returns a char* which should not be freed but the caller. */
+const char *RM_ServerInfoGetFieldC(RedisModuleServerInfoData *data, const char* field) {
+    sds val = raxFind(data->rax, (unsigned char *)field, strlen(field));
+    if (val == raxNotFound) return NULL;
+    return val;
+}
+
 /* 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(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field, int *out_err) {
-    UNUSED(ctx);
+long long RM_ServerInfoGetFieldNumerical(RedisModuleServerInfoData *data, const char* field, int *out_err) {
     long long ll;
     sds val = raxFind(data->rax, (unsigned char *)field, strlen(field));
     if (val == raxNotFound) {
@@ -5556,8 +5562,7 @@ long long RM_ServerInfoGetFieldNumerical(RedisModuleCtx *ctx, RedisModuleServerI
 /* 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(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field, int *out_err) {
-    UNUSED(ctx);
+double RM_ServerInfoGetFieldDouble(RedisModuleServerInfoData *data, const char* field, int *out_err) {
     double dbl;
     sds val = raxFind(data->rax, (unsigned char *)field, strlen(field));
     if (val == raxNotFound) {
@@ -6862,6 +6867,7 @@ void moduleRegisterCoreAPI(void) {
     REGISTER_API(GetServerInfo);
     REGISTER_API(FreeServerInfo);
     REGISTER_API(ServerInfoGetField);
+    REGISTER_API(ServerInfoGetFieldC);
     REGISTER_API(ServerInfoGetFieldNumerical);
     REGISTER_API(ServerInfoGetFieldDouble);
     REGISTER_API(GetClientInfoById);
diff --git a/src/redismodule.h b/src/redismodule.h
index d923a1b8a..0c289848f 100644
--- a/src/redismodule.h
+++ b/src/redismodule.h
@@ -506,8 +506,9 @@ int REDISMODULE_API_FUNC(RedisModule_InfoAddFieldULongLong)(RedisModuleInfoCtx *
 RedisModuleServerInfoData *REDISMODULE_API_FUNC(RedisModule_GetServerInfo)(RedisModuleCtx *ctx, const char *section);
 void REDISMODULE_API_FUNC(RedisModule_FreeServerInfo)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data);
 RedisModuleString *REDISMODULE_API_FUNC(RedisModule_ServerInfoGetField)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field);
-long long REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldNumerical)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field, int *out_err);
-double REDISMODULE_API_FUNC(RedisModule_ServerInfoGetFieldDouble)(RedisModuleCtx *ctx, RedisModuleServerInfoData *data, const char* field, int *out_err);
+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);
+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);
 void REDISMODULE_API_FUNC(RedisModule_SignalKeyAsReady)(RedisModuleCtx *ctx, RedisModuleString *key);
@@ -713,6 +714,7 @@ static int RedisModule_Init(RedisModuleCtx *ctx, const char *name, int ver, int
     REDISMODULE_GET_API(GetServerInfo);
     REDISMODULE_GET_API(FreeServerInfo);
     REDISMODULE_GET_API(ServerInfoGetField);
+    REDISMODULE_GET_API(ServerInfoGetFieldC);
     REDISMODULE_GET_API(ServerInfoGetFieldNumerical);
     REDISMODULE_GET_API(ServerInfoGetFieldDouble);
     REDISMODULE_GET_API(GetClientInfoById);
diff --git a/tests/modules/infotest.c b/tests/modules/infotest.c
index 460ebb9c0..a21fefffc 100644
--- a/tests/modules/infotest.c
+++ b/tests/modules/infotest.c
@@ -41,13 +41,17 @@ 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(ctx, info, field, &err);
+        long long ll = RedisModule_ServerInfoGetFieldNumerical(info, field, &err);
         if (err==REDISMODULE_OK)
             RedisModule_ReplyWithLongLong(ctx, ll);
     } else if (field_type=='d') {
-        double d = RedisModule_ServerInfoGetFieldDouble(ctx, info, field, &err);
+        double d = RedisModule_ServerInfoGetFieldDouble(info, field, &err);
         if (err==REDISMODULE_OK)
             RedisModule_ReplyWithDouble(ctx, d);
+    } else if (field_type=='c') {
+        const char *str = RedisModule_ServerInfoGetFieldC(info, field);
+        if (str)
+            RedisModule_ReplyWithCString(ctx, str);
     } else {
         RedisModuleString *str = RedisModule_ServerInfoGetField(ctx, info, field);
         if (str) {
@@ -66,6 +70,10 @@ int info_gets(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
     return info_get(ctx, argv, argc, 's');
 }
 
+int info_getc(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
+    return info_get(ctx, argv, argc, 'c');
+}
+
 int info_geti(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
     return info_get(ctx, argv, argc, 'i');
 }
@@ -84,6 +92,8 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 
     if (RedisModule_CreateCommand(ctx,"info.gets", info_gets,"",0,0,0) == REDISMODULE_ERR)
         return REDISMODULE_ERR;
+    if (RedisModule_CreateCommand(ctx,"info.getc", info_getc,"",0,0,0) == REDISMODULE_ERR)
+        return REDISMODULE_ERR;
     if (RedisModule_CreateCommand(ctx,"info.geti", info_geti,"",0,0,0) == REDISMODULE_ERR)
         return REDISMODULE_ERR;
     if (RedisModule_CreateCommand(ctx,"info.getd", info_getd,"",0,0,0) == REDISMODULE_ERR)
diff --git a/tests/unit/moduleapi/infotest.tcl b/tests/unit/moduleapi/infotest.tcl
index a42681345..225798dd5 100644
--- a/tests/unit/moduleapi/infotest.tcl
+++ b/tests/unit/moduleapi/infotest.tcl
@@ -13,6 +13,7 @@ start_server {tags {"modules"}} {
     test {module reading info} {
         # check string, integer and float fields
         assert_equal [r info.gets replication role] "master"
+        assert_equal [r info.getc replication role] "master"
         assert_equal [r info.geti stats expired_keys] 0
         assert_equal [r info.getd stats expired_stale_perc] 0
 

From 04233097688ee35d451c6f5cd64c28e57ca81b53 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Mon, 4 Nov 2019 08:50:29 +0200
Subject: [PATCH 3/3] Add RM_ServerInfoGetFieldUnsigned

rename RM_ServerInfoGetFieldNumerical RM_ServerInfoGetFieldSigned
move string2ull to util.c
fix leak in RM_GetServerInfo when duplicate info fields exist
---
 src/module.c                      | 30 +++++++++++++++++++++++++-----
 src/redismodule.h                 |  6 ++++--
 src/t_stream.c                    | 20 --------------------
 src/util.c                        | 20 ++++++++++++++++++++
 src/util.h                        |  1 +
 tests/modules/infotest.c          | 13 ++++++++++++-
 tests/unit/moduleapi/infotest.tcl |  4 ++++
 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) {
@@ -5559,6 +5560,24 @@ long long RM_ServerInfoGetFieldNumerical(RedisModuleServerInfoData *data, const
     return ll;
 }
 
+/* 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. */
@@ -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]