From d57120cdb6bde31848d35561366b201e00ef38c0 Mon Sep 17 00:00:00 2001 From: Ariel Date: Sun, 24 Nov 2019 14:35:35 +0200 Subject: [PATCH 1/7] Improve coverage (#734) --- Makefile | 3 ++- test.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index ce1fbbbaf..4ef09c44b 100644 --- a/Makefile +++ b/Makefile @@ -338,7 +338,8 @@ coverage: gcov make check mkdir -p tmp/lcov lcov -d . -c --exclude '/usr*' -o tmp/lcov/hiredis.info - genhtml --legend -o tmp/lcov/report tmp/lcov/hiredis.info + lcov -l tmp/lcov/hiredis.info + genhtml --legend -o tmp/lcov/report tmp/lcov/hiredis.info > /dev/null 2>&1 noopt: $(MAKE) OPTIMIZATION="" diff --git a/test.c b/test.c index 977a89be3..050d90acd 100644 --- a/test.c +++ b/test.c @@ -463,6 +463,26 @@ static void test_reply_reader(void) { freeReplyObject(root); redisReaderFree(reader); + test("Correctly parse double: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ",1.23\r\n",7); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_DOUBLE && + ((redisReply*)reply)->dval == 1.23); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Correctly parse bool: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "#t\r\n",4); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_BOOL && + ((redisReply*)reply)->integer == 1); + freeReplyObject(reply); + redisReaderFree(reader); + test("Correctly parses LLONG_MAX: "); reader = redisReaderCreate(); redisReaderFeed(reader, ":9223372036854775807\r\n",22); @@ -782,6 +802,17 @@ static void test_reply_reader(void) { !strcmp(((redisReply*)reply)->str,"3492890328409238509324850943850943825024385")); freeReplyObject(reply); redisReaderFree(reader); + + /* Test type in array. */ + test("Don't do empty allocation for empty multi bulk: "); + reader = redisReaderCreate(); + redisReaderFeed(reader,(char*)"*4\r\n#t\r\n:1\r\n,1.23\r\n_\r\n",22); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_ARRAY && + ((redisReply*)reply)->elements == 4); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { @@ -1103,6 +1134,13 @@ static void test_blocking_connection(struct config config) { test_cond(reply->len == 11) freeReplyObject(reply); + test("Test redisCommandArgv func: "); + const char *argv[3] = {"SET", "foo", "bar"}; + size_t argvlen[3] = {3, 3, 3}; + reply = redisCommandArgv(c, 3, argv, argvlen); + test_cond (reply->type == REDIS_REPLY_STATUS); + freeReplyObject(reply); + test("Can parse nil replies: "); reply = redisCommand(c,"GET nokey"); test_cond(reply->type == REDIS_REPLY_NIL) @@ -1287,25 +1325,36 @@ static void test_blocking_io_errors(struct config config) { static void test_invalid_timeout_errors(struct config config) { redisContext *c; - test("Set error when an invalid timeout usec value is given to redisConnectWithTimeout: "); + if(config.type == CONN_TCP) { + test("Set error when an invalid timeout usec value is given to redisConnectWithTimeout: "); - config.tcp.timeout.tv_sec = 0; - config.tcp.timeout.tv_usec = 10000001; + config.tcp.timeout.tv_sec = 0; + config.tcp.timeout.tv_usec = 10000001; - c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout); + c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout); - test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); - redisFree(c); + test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); + redisFree(c); - test("Set error when an invalid timeout sec value is given to redisConnectWithTimeout: "); + test("Set error when an invalid timeout sec value is given to redisConnectWithTimeout: "); - config.tcp.timeout.tv_sec = (((LONG_MAX) - 999) / 1000) + 1; - config.tcp.timeout.tv_usec = 0; + config.tcp.timeout.tv_sec = (((LONG_MAX) - 999) / 1000) + 1; + config.tcp.timeout.tv_usec = 0; - c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout); + c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout); - test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); - redisFree(c); + test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); + redisFree(c); + } else if(config.type == CONN_UNIX) { + test("Set error when an invalid timeout usec value is given to redisConnectUnixWithTimeout: "); + + config.tcp.timeout.tv_sec = 0; + config.tcp.timeout.tv_usec = 10000001; + + c = redisConnectUnixWithTimeout(config.unix_sock.path, config.tcp.timeout); + test_cond(c->err == REDIS_ERR_IO); + redisFree(c); + } } /* Wrap malloc to abort on failure so OOM checks don't make the test logic @@ -2284,6 +2333,7 @@ int main(int argc, char **argv) { test_blocking_connection(cfg); test_blocking_connection_timeouts(cfg); test_blocking_io_errors(cfg); + test_invalid_timeout_errors(cfg); if (throughput) test_throughput(cfg); } else { test_skipped(); From 3542ca22733a57c1f9ae8883075519a00f904f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 14 Oct 2022 10:39:45 +0200 Subject: [PATCH 2/7] Remove duplicate tests - double covered by: "Can parse RESP3 doubles" - bool covered via: "Can parse RESP3 bool" --- test.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/test.c b/test.c index 050d90acd..bf82b8c64 100644 --- a/test.c +++ b/test.c @@ -463,26 +463,6 @@ static void test_reply_reader(void) { freeReplyObject(root); redisReaderFree(reader); - test("Correctly parse double: "); - reader = redisReaderCreate(); - redisReaderFeed(reader, ",1.23\r\n",7); - ret = redisReaderGetReply(reader,&reply); - test_cond(ret == REDIS_OK && - ((redisReply*)reply)->type == REDIS_REPLY_DOUBLE && - ((redisReply*)reply)->dval == 1.23); - freeReplyObject(reply); - redisReaderFree(reader); - - test("Correctly parse bool: "); - reader = redisReaderCreate(); - redisReaderFeed(reader, "#t\r\n",4); - ret = redisReaderGetReply(reader,&reply); - test_cond(ret == REDIS_OK && - ((redisReply*)reply)->type == REDIS_REPLY_BOOL && - ((redisReply*)reply)->integer == 1); - freeReplyObject(reply); - redisReaderFree(reader); - test("Correctly parses LLONG_MAX: "); reader = redisReaderCreate(); redisReaderFeed(reader, ":9223372036854775807\r\n",22); From 8b039394d84179451d1d784158a8fb46e344026c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 14 Oct 2022 12:47:54 +0200 Subject: [PATCH 3/7] Make (connect) timeout in test config general --- test.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test.c b/test.c index bf82b8c64..187481903 100644 --- a/test.c +++ b/test.c @@ -35,11 +35,11 @@ enum connection_type { struct config { enum connection_type type; + struct timeval connect_timeout; struct { const char *host; int port; - struct timeval timeout; } tcp; struct { @@ -1308,30 +1308,30 @@ static void test_invalid_timeout_errors(struct config config) { if(config.type == CONN_TCP) { test("Set error when an invalid timeout usec value is given to redisConnectWithTimeout: "); - config.tcp.timeout.tv_sec = 0; - config.tcp.timeout.tv_usec = 10000001; + config.connect_timeout.tv_sec = 0; + config.connect_timeout.tv_usec = 10000001; - c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout); + c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.connect_timeout); test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); redisFree(c); test("Set error when an invalid timeout sec value is given to redisConnectWithTimeout: "); - config.tcp.timeout.tv_sec = (((LONG_MAX) - 999) / 1000) + 1; - config.tcp.timeout.tv_usec = 0; + config.connect_timeout.tv_sec = (((LONG_MAX) - 999) / 1000) + 1; + config.connect_timeout.tv_usec = 0; - c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout); + c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.connect_timeout); test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); redisFree(c); } else if(config.type == CONN_UNIX) { test("Set error when an invalid timeout usec value is given to redisConnectUnixWithTimeout: "); - config.tcp.timeout.tv_sec = 0; - config.tcp.timeout.tv_usec = 10000001; + config.connect_timeout.tv_sec = 0; + config.connect_timeout.tv_usec = 10000001; - c = redisConnectUnixWithTimeout(config.unix_sock.path, config.tcp.timeout); + c = redisConnectUnixWithTimeout(config.unix_sock.path, config.connect_timeout); test_cond(c->err == REDIS_ERR_IO); redisFree(c); } @@ -2083,11 +2083,11 @@ static redisAsyncContext *do_aconnect(struct config config, astest_no testno) if (config.type == CONN_TCP) { options.type = REDIS_CONN_TCP; - options.connect_timeout = &config.tcp.timeout; + options.connect_timeout = &config.connect_timeout; REDIS_OPTIONS_SET_TCP(&options, config.tcp.host, config.tcp.port); } else if (config.type == CONN_SSL) { options.type = REDIS_CONN_TCP; - options.connect_timeout = &config.tcp.timeout; + options.connect_timeout = &config.connect_timeout; REDIS_OPTIONS_SET_TCP(&options, config.ssl.host, config.ssl.port); } else if (config.type == CONN_UNIX) { options.type = REDIS_CONN_UNIX; @@ -2149,7 +2149,7 @@ static void test_async_polling(struct config config) { /* timeout can only be simulated with network */ test("Async connect timeout: "); config.tcp.host = "192.168.254.254"; /* blackhole ip */ - config.tcp.timeout.tv_usec = 100000; + config.connect_timeout.tv_usec = 100000; c = do_aconnect(config, ASTEST_CONN_TIMEOUT); assert(c); assert(c->err == 0); @@ -2181,7 +2181,7 @@ static void test_async_polling(struct config config) { */ if (config.type == CONN_TCP || config.type == CONN_SSL) { test("Async PING/PONG after connect timeout: "); - config.tcp.timeout.tv_usec = 10000; /* 10ms */ + config.connect_timeout.tv_usec = 10000; /* 10ms */ c = do_aconnect(config, ASTEST_PINGPONG_TIMEOUT); while(astest.connected == 0) redisPollTick(c, 0.1); From e226092ada622d49e7af5e8ea8423d290ede1fef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 14 Oct 2022 14:31:32 +0200 Subject: [PATCH 4/7] Set error string in unix connect with invalid timeout Restructure testcase since redisConnectWithTimeout() and redisConnectUnixWithTimeout() now behaves similar. --- net.c | 2 +- test.c | 39 ++++++++++++++++++++------------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/net.c b/net.c index 0f36da24a..ec96412c6 100644 --- a/net.c +++ b/net.c @@ -238,6 +238,7 @@ static int redisContextTimeoutMsec(redisContext *c, long *result) /* Only use timeout when not NULL. */ if (timeout != NULL) { if (timeout->tv_usec > 1000000 || timeout->tv_sec > __MAX_MSEC) { + __redisSetError(c, REDIS_ERR_IO, "Invalid timeout specified"); *result = msec; return REDIS_ERR; } @@ -435,7 +436,6 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port, } if (redisContextTimeoutMsec(c, &timeout_msec) != REDIS_OK) { - __redisSetError(c, REDIS_ERR_IO, "Invalid timeout specified"); goto error; } diff --git a/test.c b/test.c index 187481903..75551e0e2 100644 --- a/test.c +++ b/test.c @@ -1305,36 +1305,37 @@ static void test_blocking_io_errors(struct config config) { static void test_invalid_timeout_errors(struct config config) { redisContext *c; - if(config.type == CONN_TCP) { - test("Set error when an invalid timeout usec value is given to redisConnectWithTimeout: "); + test("Set error when an invalid timeout usec value is used during connect: "); - config.connect_timeout.tv_sec = 0; - config.connect_timeout.tv_usec = 10000001; + config.connect_timeout.tv_sec = 0; + config.connect_timeout.tv_usec = 10000001; + if (config.type == CONN_TCP || config.type == CONN_SSL) { c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.connect_timeout); + } else if(config.type == CONN_UNIX) { + c = redisConnectUnixWithTimeout(config.unix_sock.path, config.connect_timeout); + } else { + assert(NULL); + } - test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); - redisFree(c); + test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); + redisFree(c); - test("Set error when an invalid timeout sec value is given to redisConnectWithTimeout: "); + test("Set error when an invalid timeout sec value is used during connect: "); - config.connect_timeout.tv_sec = (((LONG_MAX) - 999) / 1000) + 1; - config.connect_timeout.tv_usec = 0; + config.connect_timeout.tv_sec = (((LONG_MAX) - 999) / 1000) + 1; + config.connect_timeout.tv_usec = 0; + if (config.type == CONN_TCP || config.type == CONN_SSL) { c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.connect_timeout); - - test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); - redisFree(c); } else if(config.type == CONN_UNIX) { - test("Set error when an invalid timeout usec value is given to redisConnectUnixWithTimeout: "); - - config.connect_timeout.tv_sec = 0; - config.connect_timeout.tv_usec = 10000001; - c = redisConnectUnixWithTimeout(config.unix_sock.path, config.connect_timeout); - test_cond(c->err == REDIS_ERR_IO); - redisFree(c); + } else { + assert(NULL); } + + test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0); + redisFree(c); } /* Wrap malloc to abort on failure so OOM checks don't make the test logic From 7329651a31e2a091fcb3ded4d4ce23836c1e6b75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 14 Oct 2022 14:51:18 +0200 Subject: [PATCH 5/7] Use quiet flag in lcov/genhtml instead of piping to /dev/null --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 4ef09c44b..016a77160 100644 --- a/Makefile +++ b/Makefile @@ -338,8 +338,8 @@ coverage: gcov make check mkdir -p tmp/lcov lcov -d . -c --exclude '/usr*' -o tmp/lcov/hiredis.info - lcov -l tmp/lcov/hiredis.info - genhtml --legend -o tmp/lcov/report tmp/lcov/hiredis.info > /dev/null 2>&1 + lcov -q -l tmp/lcov/hiredis.info + genhtml --legend -q -o tmp/lcov/report tmp/lcov/hiredis.info noopt: $(MAKE) OPTIMIZATION="" From 5623e1e1e782767acdc63ea2b897ca465f6fa213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 14 Oct 2022 14:59:41 +0200 Subject: [PATCH 6/7] Fixup of redisCommandArgv testcase --- test.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test.c b/test.c index 75551e0e2..6de95393f 100644 --- a/test.c +++ b/test.c @@ -1114,13 +1114,6 @@ static void test_blocking_connection(struct config config) { test_cond(reply->len == 11) freeReplyObject(reply); - test("Test redisCommandArgv func: "); - const char *argv[3] = {"SET", "foo", "bar"}; - size_t argvlen[3] = {3, 3, 3}; - reply = redisCommandArgv(c, 3, argv, argvlen); - test_cond (reply->type == REDIS_REPLY_STATUS); - freeReplyObject(reply); - test("Can parse nil replies: "); reply = redisCommand(c,"GET nokey"); test_cond(reply->type == REDIS_REPLY_NIL) @@ -1159,6 +1152,13 @@ static void test_blocking_connection(struct config config) { strcasecmp(reply->element[1]->str,"pong") == 0); freeReplyObject(reply); + test("Send command by passing argc/argv: "); + const char *argv[3] = {"SET", "foo", "bar"}; + size_t argvlen[3] = {3, 3, 3}; + reply = redisCommandArgv(c,3,argv,argvlen); + test_cond(reply->type == REDIS_REPLY_STATUS); + freeReplyObject(reply); + /* Make sure passing NULL to redisGetReply is safe */ test("Can pass NULL to redisGetReply: "); assert(redisAppendCommand(c, "PING") == REDIS_OK); From 206a897446160487578447c150fc783d3f9967b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Fri, 14 Oct 2022 15:43:39 +0200 Subject: [PATCH 7/7] Update testcase to match what it covers Use new testcase info text since the previous seemed copy&pasted. The seeked coverage was the handling of the parent-chaining for a Double object, which the testcase now focus on. --- test.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test.c b/test.c index 6de95393f..989d4bf89 100644 --- a/test.c +++ b/test.c @@ -783,14 +783,17 @@ static void test_reply_reader(void) { freeReplyObject(reply); redisReaderFree(reader); - /* Test type in array. */ - test("Don't do empty allocation for empty multi bulk: "); + test("Can parse RESP3 doubles in an array: "); reader = redisReaderCreate(); - redisReaderFeed(reader,(char*)"*4\r\n#t\r\n:1\r\n,1.23\r\n_\r\n",22); + redisReaderFeed(reader, "*1\r\n,3.14159265358979323846\r\n",31); ret = redisReaderGetReply(reader,&reply); test_cond(ret == REDIS_OK && ((redisReply*)reply)->type == REDIS_REPLY_ARRAY && - ((redisReply*)reply)->elements == 4); + ((redisReply*)reply)->elements == 1 && + ((redisReply*)reply)->element[0]->type == REDIS_REPLY_DOUBLE && + fabs(((redisReply*)reply)->element[0]->dval - 3.14159265358979323846) < 0.00000001 && + ((redisReply*)reply)->element[0]->len == 22 && + strcmp(((redisReply*)reply)->element[0]->str, "3.14159265358979323846") == 0); freeReplyObject(reply); redisReaderFree(reader); }