From 4620bfb47ad68de4b9a89c48c3978b6ad538b9ed Mon Sep 17 00:00:00 2001 From: Yuan Zhou Date: Wed, 29 May 2019 14:21:47 +0800 Subject: [PATCH 1/5] aof: fix assignment for aof_fsync_offset Signed-off-by: Yuan Zhou --- src/aof.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aof.c b/src/aof.c index 4744847d2..c8fb8e8f6 100644 --- a/src/aof.c +++ b/src/aof.c @@ -1768,7 +1768,7 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) { server.aof_selected_db = -1; /* Make sure SELECT is re-issued */ aofUpdateCurrentSize(); server.aof_rewrite_base_size = server.aof_current_size; - server.aof_current_size = server.aof_current_size; + server.aof_fsync_offset = server.aof_current_size; /* Clear regular AOF buffer since its contents was just written to * the new AOF from the background rewrite buffer. */ From 5752b1718b3b729971eb3db5d363d8537caa65da Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Tue, 29 Oct 2019 17:29:06 +0200 Subject: [PATCH 2/5] test infra: improve prints on failed assertions sometimes we have several assertions with the same condition in the same test at different stages, and when these fail (the ones that print the condition text) you don't know which one it was. other assertions didn't print the condition text (variable names), just the expected and unexpected values. So now, all assertions print context line, and conditin text. besides, one of the major differences between 'assert' and 'assert_equal', is that the later is able to print the value that doesn't match the expected. if there is a rare non-reproducible failure, it is helpful to know what was the value the test encountered and how far it was from the threshold. So now, adding assert_lessthan and assert_range that can be used in some places. were we used just 'assert { a > b }' so far. --- tests/support/test.tcl | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/tests/support/test.tcl b/tests/support/test.tcl index 2646acecd..5e8916236 100644 --- a/tests/support/test.tcl +++ b/tests/support/test.tcl @@ -11,28 +11,55 @@ proc fail {msg} { proc assert {condition} { if {![uplevel 1 [list expr $condition]]} { - error "assertion:Expected condition '$condition' to be true ([uplevel 1 [list subst -nocommands $condition]])" + set context "(context: [info frame -1])" + error "assertion:Expected [uplevel 1 [list subst -nocommands $condition]] $context" } } proc assert_no_match {pattern value} { if {[string match $pattern $value]} { - error "assertion:Expected '$value' to not match '$pattern'" + set context "(context: [info frame -1])" + error "assertion:Expected '$value' to not match '$pattern' $context" } } proc assert_match {pattern value} { if {![string match $pattern $value]} { - error "assertion:Expected '$value' to match '$pattern'" + set context "(context: [info frame -1])" + error "assertion:Expected '$value' to match '$pattern' $context" } } -proc assert_equal {expected value {detail ""}} { +proc assert_equal {value expected {detail ""}} { if {$expected ne $value} { if {$detail ne ""} { - set detail " (detail: $detail)" + set detail "(detail: $detail)" + } else { + set detail "(context: [info frame -1])" } - error "assertion:Expected '$value' to be equal to '$expected'$detail" + error "assertion:Expected '$value' to be equal to '$expected' $detail" + } +} + +proc assert_lessthan {value expected {detail ""}} { + if {!($value < $expected)} { + if {$detail ne ""} { + set detail "(detail: $detail)" + } else { + set detail "(context: [info frame -1])" + } + error "assertion:Expected '$value' to be lessthan to '$expected' $detail" + } +} + +proc assert_range {value min max {detail ""}} { + if {!($value <= $max && $value >= $min)} { + if {$detail ne ""} { + set detail "(detail: $detail)" + } else { + set detail "(context: [info frame -1])" + } + error "assertion:Expected '$value' to be between to '$min' and '$max' $detail" } } From bd206b583a58f89dd1e2743b678d148dad31174b Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 10 Nov 2019 09:21:19 +0200 Subject: [PATCH 3/5] fix leak in module api rdb test recently added more reads into that function, if a later read fails, i must either free what's already allocated, or return the pointer so that the free callback will release it. --- tests/modules/testrdb.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/modules/testrdb.c b/tests/modules/testrdb.c index 8a262e8a7..7c04bb4ef 100644 --- a/tests/modules/testrdb.c +++ b/tests/modules/testrdb.c @@ -18,8 +18,11 @@ void *testrdb_type_load(RedisModuleIO *rdb, int encver) { RedisModuleString *str = RedisModule_LoadString(rdb); float f = RedisModule_LoadFloat(rdb); long double ld = RedisModule_LoadLongDouble(rdb); - if (RedisModule_IsIOError(rdb)) + if (RedisModule_IsIOError(rdb)) { + RedisModuleCtx *ctx = RedisModule_GetContextFromIO(rdb); + RedisModule_FreeString(ctx, str); return NULL; + } /* Using the values only after checking for io errors. */ assert(count==1); assert(encver==1); From 0b6feb152481760ae549a07a8a074dc5fc58b880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=96=9C=E6=AC=A2=E5=85=B0=E8=8A=B1=E5=B1=B1=E4=B8=98?= Date: Wed, 13 Nov 2019 10:14:45 +0800 Subject: [PATCH 4/5] Update adlist.h Update listGetFree keep format consistent --- src/adlist.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/adlist.h b/src/adlist.h index c954fac87..28b9016ce 100644 --- a/src/adlist.h +++ b/src/adlist.h @@ -66,7 +66,7 @@ typedef struct list { #define listSetMatchMethod(l,m) ((l)->match = (m)) #define listGetDupMethod(l) ((l)->dup) -#define listGetFree(l) ((l)->free) +#define listGetFreeMethod(l) ((l)->free) #define listGetMatchMethod(l) ((l)->match) /* Prototypes */ From 3d4a44bd0b21d1ad9d0d8252a625a9a9f1d20664 Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 19 Nov 2019 12:10:48 +0200 Subject: [PATCH 5/5] Improve RM_Call() errno classification. RM_Call() will now use EBADF and ENONET in addition to EINVAL in order to provide more information about errors (i.e. when return value is NULL). --- src/module.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/module.c b/src/module.c index ad34e7b64..8796a7ab1 100644 --- a/src/module.c +++ b/src/module.c @@ -3110,7 +3110,9 @@ fmterr: * On success a RedisModuleCallReply object is returned, otherwise * NULL is returned and errno is set to the following values: * - * EINVAL: command non existing, wrong arity, wrong format specifier. + * EBADF: wrong format specifier. + * EINVAL: wrong command arity. + * ENOENT: command does not exist. * EPERM: operation in Cluster instance with key in non local slot. * * This API is documented here: https://redis.io/topics/modules-intro @@ -3142,7 +3144,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch /* We handle the above format error only when the client is setup so that * we can free it normally. */ if (argv == NULL) { - errno = EINVAL; + errno = EBADF; goto cleanup; } @@ -3154,7 +3156,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch */ cmd = lookupCommand(c->argv[0]->ptr); if (!cmd) { - errno = EINVAL; + errno = ENOENT; goto cleanup; } c->cmd = c->lastcmd = cmd;