From 8abfd60da98e2890ad95f3bb515f18c277bce662 Mon Sep 17 00:00:00 2001 From: Moti Cohen Date: Thu, 18 Jul 2024 10:57:08 +0300 Subject: [PATCH] PR fixes and code reorganization --- src/cli/rdb-cli.c | 55 ++++++++++++------- src/ext/handlersToJson.c | 7 +-- src/ext/handlersToPrint.c | 9 ++-- src/ext/respToFileWriter.c | 2 +- test/test_rdb_cli.c | 107 +++++++++++++++++++++++-------------- 5 files changed, 111 insertions(+), 69 deletions(-) diff --git a/src/cli/rdb-cli.c b/src/cli/rdb-cli.c index 0b20f14..ff3f0db 100644 --- a/src/cli/rdb-cli.c +++ b/src/cli/rdb-cli.c @@ -22,14 +22,19 @@ typedef struct Options { void loggerWrap(RdbLogLevel l, const char *msg, ...); -static int getOptArg(int argc, char* argv[], int *at, char *abbrvOpt, char *opt, int *flag, const char **arg) { +/* This function checks if the current argument matches the given option or its + * abbreviation. If a match is found and an argument is expected, it retrieves the + * argument and stores it. It also sets a `flag` or read `extraArg` if provided. + */ +static int getOptArg(int argc, char* argv[], int *at, char *abbrvOpt, char *opt, + int *flag, const char **extraArg) { if ((strcmp(argv[*at], abbrvOpt) == 0) || (strcmp(argv[*at], opt) == 0)) { - if (arg) { + if (extraArg) { if ((*at) + 1 == argc) { fprintf(stderr, "%s (%s) requires one argument.", opt, abbrvOpt); - exit(RDB_ERR_GENERAL); + exit(1); } - *arg = argv[++(*at)]; + *extraArg = argv[++(*at)]; } if (flag) *flag = 1; return 1; @@ -38,7 +43,13 @@ static int getOptArg(int argc, char* argv[], int *at, char *abbrvOpt, char *opt, } } -static int getOptArgVal(int argc, char* argv[], int *at, char *abbrvOpt, char *opt, int *flag, int *val, int min, int max) { +/* This function checks if the current argument matches the given option or its + * abbreviation. If a match is found, it retrieves the argument, converts it to + * an integer, and verifies that it is within the specified boundaries. It also + * sets a `flag` if provided. + */ +static int getOptArgVal(int argc, char* argv[], int *at, char *abbrvOpt, char *opt, + int *flag, int *val, int min, int max) { const char *valStr; if (getOptArg(argc, argv, at, abbrvOpt, opt, flag, &valStr)) { @@ -46,8 +57,9 @@ static int getOptArgVal(int argc, char* argv[], int *at, char *abbrvOpt, char *o /* check boundaries. Condition support also the limits INT_MAX and INT_MIN. */ if (!((*val>=min) && (*val <=max))) { - loggerWrap(RDB_LOG_ERR, "Value of %s (%s) must be a integer between %d and %d", opt, abbrvOpt, min, max); - exit(RDB_ERR_GENERAL); + loggerWrap(RDB_LOG_ERR, "Value of %s (%s) must be a integer between %d and %d", + opt, abbrvOpt, min, max); + exit(1); } return 1; } @@ -137,7 +149,8 @@ static RdbRes formatJson(RdbParser *parser, int argc, char **argv) { extern const char *jsonMetaPrefix; const char *includeArg; const char *output = NULL;/*default:stdout*/ - int includeDbInfo=0, includeStreamMeta=0, includeFunc=0, includeAuxField=0, flatten=0; /*without*/ + int includeDbInfo=0, includeStreamMeta=0, includeFunc=0, includeAuxField=0, + flatten=0; /* parse specific command options */ for (int at = 1; at < argc; ++at) { @@ -325,9 +338,10 @@ int matchRdbDataType(const char *dataTypeStr) { if (!strcmp(dataTypeStr, "stream")) return RDB_DATA_TYPE_STREAM; if (!strcmp(dataTypeStr, "func")) return RDB_DATA_TYPE_FUNCTION; - loggerWrap(RDB_LOG_ERR, "Invalid TYPE argument (%s). Valid values: str, list, set, zset, hash, module, stream, func", + loggerWrap(RDB_LOG_ERR, + "Invalid TYPE argument (%s). Valid values: str, list, set, zset, hash, module, stream, func", dataTypeStr); - exit(RDB_ERR_GENERAL); + exit(1); } int readCommonOptions(RdbParser *p, int argc, char* argv[], Options *options, int applyFilters) { @@ -348,49 +362,49 @@ int readCommonOptions(RdbParser *p, int argc, char* argv[], Options *options, in if (getOptArg(argc, argv, &at, "-k", "--key", NULL, &keyFilter)) { if (applyFilters && (!RDBX_createHandlersFilterKey(p, keyFilter, 0))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } if (getOptArg(argc, argv, &at, "-K", "--no-key", NULL, &keyFilter)) { if (applyFilters && (!RDBX_createHandlersFilterKey(p, keyFilter, 1))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } if (getOptArg(argc, argv, &at, "-t", "--type", NULL, &typeFilter)) { if ((applyFilters) && (!RDBX_createHandlersFilterType(p, matchRdbDataType(typeFilter), 0))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } if (getOptArg(argc, argv, &at, "-T", "--no-type", NULL, &typeFilter)) { if ((applyFilters) && (!RDBX_createHandlersFilterType(p, matchRdbDataType(typeFilter), 1))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } if (getOptArgVal(argc, argv, &at, "-d", "--dbnum", NULL, &dbNumFilter, 0, INT_MAX)) { if ((applyFilters) && (!RDBX_createHandlersFilterDbNum(p, dbNumFilter, 0))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } if (getOptArgVal(argc, argv, &at, "-D", "--no-dbnum", NULL, &dbNumFilter, 0, INT_MAX)) { if ((applyFilters) && (!RDBX_createHandlersFilterDbNum(p, dbNumFilter, 1))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } if (getOptArg(argc, argv, &at, "-e", "--expired", NULL, NULL)) { if ((applyFilters) && (!RDBX_createHandlersFilterExpired(p, 0))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } if (getOptArg(argc, argv, &at, "-E", "--no-expired", NULL, NULL)) { if ((applyFilters) && (!RDBX_createHandlersFilterExpired(p, 1))) - exit(RDBX_ERR_FAILED_CREATE_FILTER); + exit(1); continue; } @@ -401,7 +415,7 @@ int readCommonOptions(RdbParser *p, int argc, char* argv[], Options *options, in loggerWrap(RDB_LOG_ERR, "At argv[%d], unexpected OPTIONS argument: %s\n", at, opt); printUsage(1); - exit(RDB_ERR_GENERAL); + exit(1); } return at; } @@ -438,7 +452,8 @@ int main(int argc, char **argv) } if ((logfile = fopen(options.logfilePath, "w")) == NULL) { - printf("Error opening log file for writing `%s`: %s\n", options.logfilePath, strerror(errno)); + printf("Error opening log file for writing `%s`: %s\n", + options.logfilePath, strerror(errno)); return RDB_ERR_GENERAL; } diff --git a/src/ext/handlersToJson.c b/src/ext/handlersToJson.c index 04a0e48..46daa27 100644 --- a/src/ext/handlersToJson.c +++ b/src/ext/handlersToJson.c @@ -60,8 +60,8 @@ struct RdbxToJson { const char *jsonMetaPrefix = "__"; /* Distinct meta from data with prefix string. */ static void outputPlainEscaping(RdbxToJson *ctx, char *p, size_t len) { - while(len--) { - switch(*p) { + while (len--) { + switch (*p) { case '\\': case '"': fprintf(ctx->outfile, "\\%c", *p); break; @@ -105,7 +105,8 @@ static RdbxToJson *initRdbToJsonCtx(RdbParser *p, const char *outfilename, RdbxT outfilename = _STDOUT_STR; } else if (!(f = fopen(outfilename, "w"))) { RDB_reportError(p, RDB_ERR_FAILED_OPEN_FILE, - "HandlersRdbToJson: Failed to open file. errno=%d: %s", errno, strerror(errno)); + "HandlersRdbToJson: Failed to open file `%s`. errno=%d: %s", + outfilename, errno, strerror(errno)); return NULL; } diff --git a/src/ext/handlersToPrint.c b/src/ext/handlersToPrint.c index 3261120..a83f5d6 100644 --- a/src/ext/handlersToPrint.c +++ b/src/ext/handlersToPrint.c @@ -29,7 +29,7 @@ struct RdbxToPrint { }; static void deleteRdbToPrintCtx(RdbParser *p, void *data) { - RdbxToPrint *ctx = (RdbxToPrint *) data; + RdbxToPrint *ctx = data; RDB_bulkCopyFree(p, ctx->keyCtx.key); @@ -52,7 +52,8 @@ static RdbxToPrint *initRdbToPrintCtx(RdbParser *p, const char *auxFmt, outFilename = _STDOUT_STR; } else if (!(f = fopen(outFilename, "w"))) { RDB_reportError(p, RDB_ERR_FAILED_OPEN_FILE, - "HandlersRdbToPrint: Failed to open file. errno=%d: %s", errno, strerror(errno)); + "HandlersRdbToPrint: Failed to open file `%s`. errno=%d: %s", + outFilename, errno, strerror(errno)); return NULL; } @@ -70,8 +71,8 @@ static RdbxToPrint *initRdbToPrintCtx(RdbParser *p, const char *auxFmt, } static void outputPlainEscaping(RdbxToPrint *ctx, char *p, size_t len) { - while(len--) { - switch(*p) { + while (len--) { + switch (*p) { case '\\': case '"': fprintf(ctx->outfile, "\\%c", *p); break; diff --git a/src/ext/respToFileWriter.c b/src/ext/respToFileWriter.c index 9cdd60e..cb61c80 100644 --- a/src/ext/respToFileWriter.c +++ b/src/ext/respToFileWriter.c @@ -54,7 +54,7 @@ RdbxRespToFileWriter *RDBX_createRespToFileWriter(RdbParser *p, RdbxToResp *rdbT } else { filePtr = fopen(filePath, "wb"); if (filePtr == NULL) { - RDB_reportError(p, RDB_ERR_FAILED_OPEN_FILE, "createRespWriter: Failed to open file: %s. errno:%d", + RDB_reportError(p, RDB_ERR_FAILED_OPEN_FILE, "createRespWriter: Failed to open file: `%s`. errno:%d", filePath, errno); return NULL; } diff --git a/test/test_rdb_cli.c b/test/test_rdb_cli.c index 7bf9037..3fcfac2 100644 --- a/test/test_rdb_cli.c +++ b/test/test_rdb_cli.c @@ -1,3 +1,6 @@ +/* This file tests rdb-cli. Additionally, it also tests the 'print' formatter, + * which does not have its own separate test file. */ + #include #include #include "test_common.h" @@ -74,26 +77,29 @@ static void test_rdb_cli_resp_to_redis(void **state) { static void test_rdb_cli_filter_db(void **state) { UNUSED(state); - char *output; + /* -d/--dbnum 0 (found x but not y or z) */ - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_dbs.rdb -d 0 print -k \"%%k\" | sort"); - assert_string_equal(output, "x\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_dbs.rdb -d 0 print -k \"%%k\" | sort"), + "x\n"); /* -D/--no-dbnum 0 (found y and z but not x) */ - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_dbs.rdb --no-dbnum 0 print -k \"%%k\" | sort"); - assert_string_equal(output, "y\nz\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_dbs.rdb --no-dbnum 0 print -k \"%%k\" | sort"), + "y\nz\n"); } static void test_rdb_cli_filter_key(void **state) { - char *output; UNUSED(state); /* -k/--key (found string2 but not mylist1 or lzf_compressed) */ - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -k string2 print -k \"%%k\" | sort"); - assert_string_equal(output, "string2\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -k string2 print -k \"%%k\" | sort"), + "string2\n"); /* -K/--no-key (found mylist1 or lzf_compressed but not string2) */ - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -K string2 print -k \"%%k\" | sort"); - assert_string_equal(output, "lzf_compressed\nmylist1\nmylist2\nmylist3\nstring1\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -K string2 print -k \"%%k\" | sort"), + "lzf_compressed\nmylist1\nmylist2\nmylist3\nstring1\n"); } static void test_rdb_cli_filter_invalid_input(void **state) { @@ -104,55 +110,68 @@ static void test_rdb_cli_filter_invalid_input(void **state) { static void test_rdb_cli_filter_type(void **state) { UNUSED(state); - char *output; + /* -t/--type */ - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb --type str print -k \"%%k\" | sort"); - assert_string_equal(output, "lzf_compressed\nstring1\nstring2\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb --type str print -k \"%%k\" | sort"), + "lzf_compressed\nstring1\nstring2\n"); - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -t str print -k \"%%k\" | sort"); - assert_string_equal(output, "lzf_compressed\nstring1\nstring2\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -t str print -k \"%%k\" | sort"), + "lzf_compressed\nstring1\nstring2\n"); /* -T/--no-type */ - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb --no-type str print -k \"%%k\" | sort"); - assert_string_equal(output, "mylist1\nmylist2\nmylist3\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb --no-type str print -k \"%%k\" | sort"), + "mylist1\nmylist2\nmylist3\n"); - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -T str print -k \"%%k\" | sort"); - assert_string_equal(output, "mylist1\nmylist2\nmylist3\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -T str print -k \"%%k\" | sort"), + "mylist1\nmylist2\nmylist3\n"); } static void test_rdb_cli_filter_expire(void **state) { UNUSED(state); - char *output; sendRedisCmd("SET persistKey STAM", REDIS_REPLY_STATUS, NULL); sendRedisCmd("SET volatileKey XXX", REDIS_REPLY_STATUS, NULL); sendRedisCmd("HSET persistHash f1 v1", REDIS_REPLY_INTEGER, "1"); sendRedisCmd("SAVE", REDIS_REPLY_STATUS, NULL); sendRedisCmd("PEXPIRE volatileKey 10", REDIS_REPLY_INTEGER, "1"); sendRedisCmd("SAVE", REDIS_REPLY_STATUS, NULL); - runSystemCmd("cp %s %s > /dev/null", TMP_FOLDER("dump.rdb"), TMP_FOLDER("test_rdb_cli_print.rdb")); + runSystemCmd("cp %s %s > /dev/null", TMP_FOLDER("dump.rdb"), TMP_FOLDER("with_expires.rdb")); usleep(20000); /*20ms*/ - output = runSystemCmd("$RDB_CLI_CMD %s print -k \"%%k\" | sort", TMP_FOLDER("test_rdb_cli_print.rdb")); - assert_string_equal(output, "persistHash\npersistKey\nvolatileKey\n"); - output = runSystemCmd("$RDB_CLI_CMD %s -e print -k \"%%k\" | sort", TMP_FOLDER("test_rdb_cli_print.rdb")); - assert_string_equal(output, "volatileKey\n"); - output = runSystemCmd("$RDB_CLI_CMD %s -E print | sort", TMP_FOLDER("test_rdb_cli_print.rdb")); - assert_string_equal(output, "0,persistHash,{...},hash,-1,1\n0,persistKey,STAM,string,-1,0\n"); + + assert_string_equal( + runSystemCmd("$RDB_CLI_CMD %s print -k \"%%k\" | sort", TMP_FOLDER("with_expires.rdb")), + "persistHash\npersistKey\nvolatileKey\n"); + + assert_string_equal( + runSystemCmd("$RDB_CLI_CMD %s -e print -k \"%%k\" | sort", TMP_FOLDER("with_expires.rdb")), + "volatileKey\n"); + + assert_string_equal( + runSystemCmd("$RDB_CLI_CMD %s -E print | sort", TMP_FOLDER("with_expires.rdb")), + "0,persistHash,{...},hash,-1,1\n0,persistKey,STAM,string,-1,0\n"); } static void test_rdb_cli_filter_mix(void **state) { UNUSED(state); - char *output; + /* Combine 'type' and 'key' filters */ - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb --type str --key string print -k \"%%k\" | sort"); - assert_string_equal(output, "string1\nstring2\n"); - output = runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -t str -k string print -k \"%%k\" | sort"); - assert_string_equal(output, "string1\nstring2\n"); + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb --type str --key string print -k \"%%k\" | sort"), + "string1\nstring2\n"); + + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_lists_strings.rdb -t str -k string print -k \"%%k\" | sort"), + "string1\nstring2\n"); } static void test_rdb_cli_input_fd_reader(void **state) { UNUSED(state); - char *out = runSystemCmd(" cat ./test/dumps/single_key.rdb | $RDB_CLI_CMD - print -k \"%%k\" | sort"); - assert_string_equal(out, "xxx\n"); + assert_string_equal( + runSystemCmd(" cat ./test/dumps/single_key.rdb | $RDB_CLI_CMD - print -k \"%%k\" | sort"), + "xxx\n"); } static void test_rdb_cli_redis_auth(void **state) { @@ -186,18 +205,24 @@ static void test_rdb_cli_redis_auth(void **state) { static void test_rdb_cli_print(void **state) { UNUSED(state); - char *output; sendRedisCmd("SET ABC STAM", REDIS_REPLY_STATUS, NULL); sendRedisCmd("HSET myhash f1 v1", REDIS_REPLY_INTEGER, "1"); sendRedisCmd("SAVE", REDIS_REPLY_STATUS, NULL); /* Check default ouput of print */ - output = runSystemCmd("$RDB_CLI_CMD %s print | sort", TMP_FOLDER("dump.rdb")); - assert_string_equal(output, "0,ABC,STAM,string,-1,0\n0,myhash,{...},hash,-1,1\n"); - - /* Check customized output of print */ - output = runSystemCmd("$RDB_CLI_CMD %s print -k \"%%k\" | sort", TMP_FOLDER("dump.rdb")); - assert_string_equal(output, "ABC\nmyhash\n"); + assert_string_equal( + runSystemCmd("$RDB_CLI_CMD %s print | sort", TMP_FOLDER("dump.rdb")), + "0,ABC,STAM,string,-1,0\n0,myhash,{...},hash,-1,1\n"); + + /* Check customized output */ + assert_string_equal( + runSystemCmd("$RDB_CLI_CMD %s print -k \"%%k\" | sort", TMP_FOLDER("dump.rdb")), + "ABC\nmyhash\n"); + + /* Check customized aux-val output */ + assert_string_equal( + runSystemCmd(" $RDB_CLI_CMD ./test/dumps/multiple_dbs.rdb print -k \"\" -a \"%%f=%%v\""), + "redis-ver=255.255.255\nredis-bits=64\nctime=1683103535\nused-mem=967040\naof-base=0\n"); } /*************************** group_test_rdb_cli *******************************/