From a6f4296a293b84c64d92a7b4efbe9398afb0f5cf Mon Sep 17 00:00:00 2001 From: LiiNen Date: Mon, 1 Apr 2024 23:37:56 +0900 Subject: [PATCH 01/11] Fix to allow end as optional when using bitcount Signed-off-by: LiiNen --- src/bitops.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/bitops.c b/src/bitops.c index 925c2a71dd..437f47571d 100644 --- a/src/bitops.c +++ b/src/bitops.c @@ -792,7 +792,7 @@ void bitopCommand(client *c) { addReplyLongLong(c,maxlen); /* Return the output string length in bytes. */ } -/* BITCOUNT key [start end [BIT|BYTE]] */ +/* BITCOUNT key [start [end [BIT|BYTE]]] */ void bitcountCommand(client *c) { robj *o; long long start, end; @@ -803,11 +803,13 @@ void bitcountCommand(client *c) { unsigned char first_byte_neg_mask = 0, last_byte_neg_mask = 0; /* Parse start/end range if any. */ - if (c->argc == 4 || c->argc == 5) { + if (c-> argc == 3 || c->argc == 4 || c->argc == 5) { if (getLongLongFromObjectOrReply(c,c->argv[2],&start,NULL) != C_OK) return; - if (getLongLongFromObjectOrReply(c,c->argv[3],&end,NULL) != C_OK) - return; + if (c->argc >= 4) { + if (getLongLongFromObjectOrReply(c,c->argv[3],&end,NULL) != C_OK) + return; + } if (c->argc == 5) { if (!strcasecmp(c->argv[4]->ptr,"bit")) isbit = 1; else if (!strcasecmp(c->argv[4]->ptr,"byte")) isbit = 0; @@ -825,12 +827,18 @@ void bitcountCommand(client *c) { /* Make sure we will not overflow */ serverAssert(totlen <= LLONG_MAX >> 3); + if (c->argc < 4) { + if (isbit) end = (totlen<<3) + 7; + else end = totlen-1; + } + /* Convert negative indexes */ if (start < 0 && end < 0 && start > end) { addReply(c,shared.czero); return; } if (isbit) totlen <<= 3; + /* Convert negative indexes */ if (start < 0) start = totlen+start; if (end < 0) end = totlen+end; if (start < 0) start = 0; @@ -849,6 +857,7 @@ void bitcountCommand(client *c) { o = lookupKeyRead(c->db, c->argv[1]); if (checkType(c, o, OBJ_STRING)) return; p = getObjectReadOnlyString(o,&strlen,llbuf); + /* The whole string. */ start = 0; end = strlen-1; From 5b067d33e824b0951f3b8b4f93b8bb10a5f4ddce Mon Sep 17 00:00:00 2001 From: LiiNen Date: Mon, 1 Apr 2024 23:41:23 +0900 Subject: [PATCH 02/11] Fix hint for bitcount Signed-off-by: LiiNen --- src/commands/bitcount.json | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/commands/bitcount.json b/src/commands/bitcount.json index 2d277a8551..616e9fa02a 100644 --- a/src/commands/bitcount.json +++ b/src/commands/bitcount.json @@ -54,24 +54,31 @@ "type": "integer" }, { - "name": "end", - "type": "integer" - }, - { - "name": "unit", - "type": "oneof", + "name": "end-unit-block", + "type": "block", "optional": true, - "since": "7.0.0", "arguments": [ { - "name": "byte", - "type": "pure-token", - "token": "BYTE" + "name": "end", + "type": "integer" }, { - "name": "bit", - "type": "pure-token", - "token": "BIT" + "name": "unit", + "type": "oneof", + "optional": true, + "since": "7.0.0", + "arguments": [ + { + "name": "byte", + "type": "pure-token", + "token": "BYTE" + }, + { + "name": "bit", + "type": "pure-token", + "token": "BIT" + } + ] } ] } From 8e813f3cf8573d86bf4eaaf2ab1853b2fcb617b3 Mon Sep 17 00:00:00 2001 From: LiiNen Date: Mon, 1 Apr 2024 23:42:39 +0900 Subject: [PATCH 03/11] Add test in case for only using start in bitcount Signed-off-by: LiiNen --- tests/unit/bitops.tcl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl index f50f65dfa0..cbd5c612a7 100644 --- a/tests/unit/bitops.tcl +++ b/tests/unit/bitops.tcl @@ -128,6 +128,15 @@ start_server {tags {"bitops"}} { } } + test {BITCOUNT with start} { + set s "foobar" + r set s $s + assert_equal [r bitcount s 0] [count_bits "foobar"] + assert_equal [r bitcount s 1] [count_bits "oobar"] + assert_equal [r bitcount s -1] [count_bits "r"] + assert_equal [r bitcount s -2] [count_bits "ar"] + } + test {BITCOUNT with start, end} { set s "foobar" r set s $s From 64008bba798091dc03295e7233f5797f480f75cf Mon Sep 17 00:00:00 2001 From: LiiNen Date: Mon, 1 Apr 2024 23:51:13 +0900 Subject: [PATCH 04/11] Remove unreachable code in BITCOUNT and BITPOS in case of end is not declared, BYTE|BIT cannot be used, so variable 'isbit' is always 0 Signed-off-by: LiiNen --- src/bitops.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/bitops.c b/src/bitops.c index 437f47571d..fa21f08172 100644 --- a/src/bitops.c +++ b/src/bitops.c @@ -806,10 +806,6 @@ void bitcountCommand(client *c) { if (c-> argc == 3 || c->argc == 4 || c->argc == 5) { if (getLongLongFromObjectOrReply(c,c->argv[2],&start,NULL) != C_OK) return; - if (c->argc >= 4) { - if (getLongLongFromObjectOrReply(c,c->argv[3],&end,NULL) != C_OK) - return; - } if (c->argc == 5) { if (!strcasecmp(c->argv[4]->ptr,"bit")) isbit = 1; else if (!strcasecmp(c->argv[4]->ptr,"byte")) isbit = 0; @@ -818,6 +814,14 @@ void bitcountCommand(client *c) { return; } } + if (c->argc >= 4) { + if (getLongLongFromObjectOrReply(c,c->argv[3],&end,NULL) != C_OK) + return; + } + else if (c->argc < 4) { + end = totlen-1; + } + /* Lookup, check for type. */ o = lookupKeyRead(c->db, c->argv[1]); if (checkType(c, o, OBJ_STRING)) return; @@ -827,11 +831,6 @@ void bitcountCommand(client *c) { /* Make sure we will not overflow */ serverAssert(totlen <= LLONG_MAX >> 3); - if (c->argc < 4) { - if (isbit) end = (totlen<<3) + 7; - else end = totlen-1; - } - /* Convert negative indexes */ if (start < 0 && end < 0 && start > end) { addReply(c,shared.czero); @@ -929,6 +928,9 @@ void bitposCommand(client *c) { return; end_given = 1; } + else if (c->argc < 5) { + end = totlen-1; + } /* Lookup, check for type. */ o = lookupKeyRead(c->db, c->argv[1]); @@ -939,11 +941,6 @@ void bitposCommand(client *c) { long long totlen = strlen; serverAssert(totlen <= LLONG_MAX >> 3); - if (c->argc < 5) { - if (isbit) end = (totlen<<3) + 7; - else end = totlen-1; - } - if (isbit) totlen <<= 3; /* Convert negative indexes */ if (start < 0) start = totlen+start; From fb2e403ced505344b482fbe90bf6acf6b7c8a942 Mon Sep 17 00:00:00 2001 From: LiiNen Date: Mon, 1 Apr 2024 23:56:14 +0900 Subject: [PATCH 05/11] Move line end = totlen - 1, after totlen declared Signed-off-by: LiiNen --- src/bitops.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/bitops.c b/src/bitops.c index fa21f08172..035840287f 100644 --- a/src/bitops.c +++ b/src/bitops.c @@ -818,9 +818,6 @@ void bitcountCommand(client *c) { if (getLongLongFromObjectOrReply(c,c->argv[3],&end,NULL) != C_OK) return; } - else if (c->argc < 4) { - end = totlen-1; - } /* Lookup, check for type. */ o = lookupKeyRead(c->db, c->argv[1]); @@ -831,13 +828,14 @@ void bitcountCommand(client *c) { /* Make sure we will not overflow */ serverAssert(totlen <= LLONG_MAX >> 3); + if (c->argc < 4) end = totlen-1; + /* Convert negative indexes */ if (start < 0 && end < 0 && start > end) { addReply(c,shared.czero); return; } if (isbit) totlen <<= 3; - /* Convert negative indexes */ if (start < 0) start = totlen+start; if (end < 0) end = totlen+end; if (start < 0) start = 0; @@ -928,9 +926,6 @@ void bitposCommand(client *c) { return; end_given = 1; } - else if (c->argc < 5) { - end = totlen-1; - } /* Lookup, check for type. */ o = lookupKeyRead(c->db, c->argv[1]); @@ -941,6 +936,8 @@ void bitposCommand(client *c) { long long totlen = strlen; serverAssert(totlen <= LLONG_MAX >> 3); + if (c->argc < 5) end = totlen-1; + if (isbit) totlen <<= 3; /* Convert negative indexes */ if (start < 0) start = totlen+start; From c7581126f90ae49843a12aca3b537cf55a68a5b0 Mon Sep 17 00:00:00 2001 From: LiiNen Date: Tue, 2 Apr 2024 00:06:24 +0900 Subject: [PATCH 06/11] Add commands.def after build Signed-off-by: LiiNen --- src/commands.def | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/commands.def b/src/commands.def index 0a86f44bab..8ab8aa0ead 100644 --- a/src/commands.def +++ b/src/commands.def @@ -51,23 +51,28 @@ keySpec BITCOUNT_Keyspecs[1] = { }; #endif -/* BITCOUNT range unit argument table */ -struct COMMAND_ARG BITCOUNT_range_unit_Subargs[] = { +/* BITCOUNT range end_unit_block unit argument table */ +struct COMMAND_ARG BITCOUNT_range_end_unit_block_unit_Subargs[] = { {MAKE_ARG("byte",ARG_TYPE_PURE_TOKEN,-1,"BYTE",NULL,NULL,CMD_ARG_NONE,0,NULL)}, {MAKE_ARG("bit",ARG_TYPE_PURE_TOKEN,-1,"BIT",NULL,NULL,CMD_ARG_NONE,0,NULL)}, }; +/* BITCOUNT range end_unit_block argument table */ +struct COMMAND_ARG BITCOUNT_range_end_unit_block_Subargs[] = { +{MAKE_ARG("end",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, +{MAKE_ARG("unit",ARG_TYPE_ONEOF,-1,NULL,NULL,"7.0.0",CMD_ARG_OPTIONAL,2,NULL),.subargs=BITCOUNT_range_end_unit_block_unit_Subargs}, +}; + /* BITCOUNT range argument table */ struct COMMAND_ARG BITCOUNT_range_Subargs[] = { {MAKE_ARG("start",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, -{MAKE_ARG("end",ARG_TYPE_INTEGER,-1,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, -{MAKE_ARG("unit",ARG_TYPE_ONEOF,-1,NULL,NULL,"7.0.0",CMD_ARG_OPTIONAL,2,NULL),.subargs=BITCOUNT_range_unit_Subargs}, +{MAKE_ARG("end-unit-block",ARG_TYPE_BLOCK,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,2,NULL),.subargs=BITCOUNT_range_end_unit_block_Subargs}, }; /* BITCOUNT argument table */ struct COMMAND_ARG BITCOUNT_Args[] = { {MAKE_ARG("key",ARG_TYPE_KEY,0,NULL,NULL,NULL,CMD_ARG_NONE,0,NULL)}, -{MAKE_ARG("range",ARG_TYPE_BLOCK,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,3,NULL),.subargs=BITCOUNT_range_Subargs}, +{MAKE_ARG("range",ARG_TYPE_BLOCK,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,2,NULL),.subargs=BITCOUNT_range_Subargs}, }; /********** BITFIELD ********************/ From fc369bb8eaeda8f80baf20e9268d7a58d3e9c29c Mon Sep 17 00:00:00 2001 From: LiiNen Date: Tue, 2 Apr 2024 11:59:50 +0900 Subject: [PATCH 07/11] Add history for this fix Signed-off-by: LiiNen --- src/commands/bitcount.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/commands/bitcount.json b/src/commands/bitcount.json index 616e9fa02a..ad90b39bad 100644 --- a/src/commands/bitcount.json +++ b/src/commands/bitcount.json @@ -10,6 +10,10 @@ [ "7.0.0", "Added the `BYTE|BIT` option." + ], + [ + "8.0.0", + "`end` made optional; when called without argument the command reports the last BYTE." ] ], "command_flags": [ From d5f0da62ae7d598be8d8d74287bff00abb35bfa0 Mon Sep 17 00:00:00 2001 From: LiiNen Date: Tue, 2 Apr 2024 12:05:58 +0900 Subject: [PATCH 08/11] Add commands.def after build Signed-off-by: LiiNen --- src/commands.def | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands.def b/src/commands.def index 8ab8aa0ead..baa289b7a3 100644 --- a/src/commands.def +++ b/src/commands.def @@ -36,6 +36,7 @@ const char *commandGroupStr(int index) { /* BITCOUNT history */ commandHistory BITCOUNT_History[] = { {"7.0.0","Added the `BYTE|BIT` option."}, +{"8.0.0","`end` made optional; when called without argument the command reports the last BYTE."}, }; #endif @@ -10650,7 +10651,7 @@ struct COMMAND_ARG WATCH_Args[] = { /* Main command table */ struct COMMAND_STRUCT redisCommandTable[] = { /* bitmap */ -{MAKE_CMD("bitcount","Counts the number of set bits (population counting) in a string.","O(N)","2.6.0",CMD_DOC_NONE,NULL,NULL,"bitmap",COMMAND_GROUP_BITMAP,BITCOUNT_History,1,BITCOUNT_Tips,0,bitcountCommand,-2,CMD_READONLY,ACL_CATEGORY_BITMAP,BITCOUNT_Keyspecs,1,NULL,2),.args=BITCOUNT_Args}, +{MAKE_CMD("bitcount","Counts the number of set bits (population counting) in a string.","O(N)","2.6.0",CMD_DOC_NONE,NULL,NULL,"bitmap",COMMAND_GROUP_BITMAP,BITCOUNT_History,2,BITCOUNT_Tips,0,bitcountCommand,-2,CMD_READONLY,ACL_CATEGORY_BITMAP,BITCOUNT_Keyspecs,1,NULL,2),.args=BITCOUNT_Args}, {MAKE_CMD("bitfield","Performs arbitrary bitfield integer operations on strings.","O(1) for each subcommand specified","3.2.0",CMD_DOC_NONE,NULL,NULL,"bitmap",COMMAND_GROUP_BITMAP,BITFIELD_History,0,BITFIELD_Tips,0,bitfieldCommand,-2,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_BITMAP,BITFIELD_Keyspecs,1,bitfieldGetKeys,2),.args=BITFIELD_Args}, {MAKE_CMD("bitfield_ro","Performs arbitrary read-only bitfield integer operations on strings.","O(1) for each subcommand specified","6.0.0",CMD_DOC_NONE,NULL,NULL,"bitmap",COMMAND_GROUP_BITMAP,BITFIELD_RO_History,0,BITFIELD_RO_Tips,0,bitfieldroCommand,-2,CMD_READONLY|CMD_FAST,ACL_CATEGORY_BITMAP,BITFIELD_RO_Keyspecs,1,NULL,2),.args=BITFIELD_RO_Args}, {MAKE_CMD("bitop","Performs bitwise operations on multiple strings, and stores the result.","O(N)","2.6.0",CMD_DOC_NONE,NULL,NULL,"bitmap",COMMAND_GROUP_BITMAP,BITOP_History,0,BITOP_Tips,0,bitopCommand,-4,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_BITMAP,BITOP_Keyspecs,2,NULL,3),.args=BITOP_Args}, From 729ec8b39f54f2c51f0dccb09c9da788b476cc65 Mon Sep 17 00:00:00 2001 From: LiiNen Date: Tue, 2 Apr 2024 12:06:56 +0900 Subject: [PATCH 09/11] Update tests/unit/bitops.tcl Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com> Signed-off-by: LiiNen --- tests/unit/bitops.tcl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl index cbd5c612a7..60035b4dbd 100644 --- a/tests/unit/bitops.tcl +++ b/tests/unit/bitops.tcl @@ -128,7 +128,7 @@ start_server {tags {"bitops"}} { } } - test {BITCOUNT with start} { + test {BITCOUNT with just start} { set s "foobar" r set s $s assert_equal [r bitcount s 0] [count_bits "foobar"] From c0dd4eea15e20420e0b4c52b7cb770a0f6e73861 Mon Sep 17 00:00:00 2001 From: LiiNen Date: Wed, 3 Apr 2024 01:59:35 +0900 Subject: [PATCH 10/11] Fix test code : allowing bitcount without end now Signed-off-by: LiiNen --- tests/unit/bitops.tcl | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl index 60035b4dbd..9be2f517f3 100644 --- a/tests/unit/bitops.tcl +++ b/tests/unit/bitops.tcl @@ -159,12 +159,10 @@ start_server {tags {"bitops"}} { test {BITCOUNT with illegal arguments} { # Used to return 0 for non-existing key instead of errors r del s - assert_error {ERR *syntax*} {r bitcount s 0} assert_error {ERR *syntax*} {r bitcount s 0 1 hello} assert_error {ERR *syntax*} {r bitcount s 0 1 hello hello2} r set s 1 - assert_error {ERR *syntax*} {r bitcount s 0} assert_error {ERR *syntax*} {r bitcount s 0 1 hello} assert_error {ERR *syntax*} {r bitcount s 0 1 hello hello2} } From 843c84a09142dad2ecf938e292053828befbe39e Mon Sep 17 00:00:00 2001 From: LiiNen Date: Mon, 29 Apr 2024 23:41:39 +0900 Subject: [PATCH 11/11] Add test cases; larger value, negative larger value Signed-off-by: LiiNen --- tests/unit/bitops.tcl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl index 9be2f517f3..03495dd8ff 100644 --- a/tests/unit/bitops.tcl +++ b/tests/unit/bitops.tcl @@ -133,8 +133,10 @@ start_server {tags {"bitops"}} { r set s $s assert_equal [r bitcount s 0] [count_bits "foobar"] assert_equal [r bitcount s 1] [count_bits "oobar"] + assert_equal [r bitcount s 1000] 0 assert_equal [r bitcount s -1] [count_bits "r"] assert_equal [r bitcount s -2] [count_bits "ar"] + assert_equal [r bitcount s -1000] [count_bits "foobar"] } test {BITCOUNT with start, end} { @@ -142,8 +144,10 @@ start_server {tags {"bitops"}} { r set s $s assert_equal [r bitcount s 0 -1] [count_bits "foobar"] assert_equal [r bitcount s 1 -2] [count_bits "ooba"] - assert_equal [r bitcount s -2 1] [count_bits ""] + assert_equal [r bitcount s -2 1] 0 + assert_equal [r bitcount s -1000 0] [count_bits "f"] assert_equal [r bitcount s 0 1000] [count_bits "foobar"] + assert_equal [r bitcount s -1000 1000] [count_bits "foobar"] assert_equal [r bitcount s 0 -1 bit] [count_bits $s] assert_equal [r bitcount s 10 14 bit] [count_bits_start_end $s 10 14] @@ -153,7 +157,9 @@ start_server {tags {"bitops"}} { assert_equal [r bitcount s 3 -34 bit] [count_bits_start_end $s 3 14] assert_equal [r bitcount s 3 -19 bit] [count_bits_start_end $s 3 29] assert_equal [r bitcount s -2 1 bit] 0 + assert_equal [r bitcount s -1000 14 bit] [count_bits_start_end $s 0 14] assert_equal [r bitcount s 0 1000 bit] [count_bits $s] + assert_equal [r bitcount s -1000 1000 bit] [count_bits $s] } test {BITCOUNT with illegal arguments} {