From 412eb67d21705ca31162d614fb8e3a6a568ef5ed 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 a0cfd519e3cbbb4c432d25094e4cb202631f131f 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 02f21113ab6c61d2c01544607b464eb501dbe8fa 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 253d9d6d12c5a19ae8faa7632068772845b4a552 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 2d1e893b3e133dcceecef2110d2c41aa8f904b87 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;