From 76ad23d012f194efa1acc0f8356d945b07681851 Mon Sep 17 00:00:00 2001 From: Itamar Haber Date: Thu, 7 Jun 2018 18:34:58 +0300 Subject: [PATCH 1/5] Adds MODULE HELP and implements addReplySubSyntaxError --- src/module.c | 13 +++++++++++-- src/networking.c | 12 ++++++++++++ src/server.h | 1 + 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/module.c b/src/module.c index cb03ad2c..e4d6e2cf 100644 --- a/src/module.c +++ b/src/module.c @@ -4499,7 +4499,15 @@ int moduleUnload(sds name) { * MODULE LOAD [args...] */ void moduleCommand(client *c) { char *subcmd = c->argv[1]->ptr; - + if (c->argc == 2 && !strcasecmp(subcmd,"help")) { + const char *help[] = { +"list -- Return a list of loaded modules.", +"load [arg ...] -- Load a module library from .", +"unload -- Unload a module.", +NULL + }; + addReplyHelp(c, help); + } else if (!strcasecmp(subcmd,"load") && c->argc >= 3) { robj **argv = NULL; int argc = 0; @@ -4548,7 +4556,8 @@ void moduleCommand(client *c) { } dictReleaseIterator(di); } else { - addReply(c,shared.syntaxerr); + addReplySubSyntaxError(c); + return; } } diff --git a/src/networking.c b/src/networking.c index 00558974..ac28ba2b 100644 --- a/src/networking.c +++ b/src/networking.c @@ -560,6 +560,18 @@ void addReplyHelp(client *c, const char **help) { setDeferredMultiBulkLength(c,blenp,blen); } +/* Add a suggestive error reply. + * This function is typically invoked by from commands that support + * subcommands in response to an unknown subcommand or argument error. */ +void addReplySubSyntaxError(client *c) { + sds cmd = sdsnew((char*) c->argv[0]->ptr); + sdstoupper(cmd); + addReplyErrorFormat(c, + "Unknown subcommand or wrong number of arguments for '%s'. Try %s HELP.", + c->argv[1]->ptr,cmd); + sdsfree(cmd); +} + /* Copy 'src' client output buffers into 'dst' client output buffers. * The function takes care of freeing the old output buffers of the * destination client. */ diff --git a/src/server.h b/src/server.h index c34cdcfb..26aee893 100644 --- a/src/server.h +++ b/src/server.h @@ -1410,6 +1410,7 @@ void addReplyHumanLongDouble(client *c, long double d); void addReplyLongLong(client *c, long long ll); void addReplyMultiBulkLen(client *c, long length); void addReplyHelp(client *c, const char **help); +void addReplySubSyntaxError(client *c); void copyClientOutputBuffer(client *dst, client *src); size_t sdsZmallocSize(sds s); size_t getStringObjectSdsUsedMemory(robj *o); From c199280edb7ad344bb3d2af7572469f74d506da7 Mon Sep 17 00:00:00 2001 From: Itamar Haber Date: Thu, 7 Jun 2018 18:39:36 +0300 Subject: [PATCH 2/5] Globally applies addReplySubSyntaxError --- src/cluster.c | 3 +-- src/config.c | 3 +-- src/debug.c | 3 +-- src/object.c | 2 +- src/pubsub.c | 3 +-- src/scripting.c | 2 +- src/server.c | 2 +- src/slowlog.c | 2 +- 8 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 17ba6a74..56deb927 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -4746,8 +4746,7 @@ NULL clusterReset(hard); addReply(c,shared.ok); } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try CLUSTER HELP", - (char*)c->argv[1]->ptr); + addReplySubSyntaxError(c); return; } } diff --git a/src/config.c b/src/config.c index bfb31030..4e6438e8 100644 --- a/src/config.c +++ b/src/config.c @@ -2149,8 +2149,7 @@ NULL addReply(c,shared.ok); } } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try CONFIG HELP", - (char*)c->argv[1]->ptr); + addReplySubSyntaxError(c); return; } } diff --git a/src/debug.c b/src/debug.c index 0ab864e7..0c5ea9a9 100644 --- a/src/debug.c +++ b/src/debug.c @@ -553,8 +553,7 @@ NULL clearReplicationId2(); addReply(c,shared.ok); } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try DEBUG HELP", - (char*)c->argv[1]->ptr); + addReplySubSyntaxError(c); return; } } diff --git a/src/object.c b/src/object.c index 19af1ec0..a4942fbe 100644 --- a/src/object.c +++ b/src/object.c @@ -1197,7 +1197,7 @@ NULL * when the key is read or overwritten. */ addReplyLongLong(c,LFUDecrAndReturn(o)); } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try OBJECT help", (char *)c->argv[1]->ptr); + addReplySubSyntaxError(c); } } diff --git a/src/pubsub.c b/src/pubsub.c index d1fffa20..7f7be52d 100644 --- a/src/pubsub.c +++ b/src/pubsub.c @@ -372,7 +372,6 @@ NULL /* PUBSUB NUMPAT */ addReplyLongLong(c,listLength(server.pubsub_patterns)); } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try PUBSUB HELP", - (char*)c->argv[1]->ptr); + addReplySubSyntaxError(c); } } diff --git a/src/scripting.c b/src/scripting.c index 3c0597c7..85bce7be 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -1514,7 +1514,7 @@ NULL return; } } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try SCRIPT HELP", (char*)c->argv[1]->ptr); + addReplySubSyntaxError(c); } } diff --git a/src/server.c b/src/server.c index 375c6477..1e4d54b8 100644 --- a/src/server.c +++ b/src/server.c @@ -2866,7 +2866,7 @@ NULL for (j = 0; j < numkeys; j++) addReplyBulk(c,c->argv[keys[j]+2]); getKeysFreeResult(keys); } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try COMMAND HELP", (char*)c->argv[1]->ptr); + addReplySubSyntaxError(c); } } diff --git a/src/slowlog.c b/src/slowlog.c index 2613435a..aed5707d 100644 --- a/src/slowlog.c +++ b/src/slowlog.c @@ -187,6 +187,6 @@ NULL } setDeferredMultiBulkLength(c,totentries,sent); } else { - addReplyErrorFormat(c, "Unknown subcommand or wrong number of arguments for '%s'. Try SLOWLOG HELP", (char*)c->argv[1]->ptr); + addReplySubSyntaxError(c); } } From 21ef0376feaaacee22b7913cb08948f65f9b8198 Mon Sep 17 00:00:00 2001 From: Itamar Haber Date: Sat, 9 Jun 2018 20:54:05 +0300 Subject: [PATCH 3/5] Capitalizes subscommands --- src/module.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/module.c b/src/module.c index e4d6e2cf..3aaa94fe 100644 --- a/src/module.c +++ b/src/module.c @@ -4501,9 +4501,9 @@ void moduleCommand(client *c) { char *subcmd = c->argv[1]->ptr; if (c->argc == 2 && !strcasecmp(subcmd,"help")) { const char *help[] = { -"list -- Return a list of loaded modules.", -"load [arg ...] -- Load a module library from .", -"unload -- Unload a module.", +"LIST -- Return a list of loaded modules.", +"LOAD [arg ...] -- Load a module library from .", +"UNLOAD -- Unload a module.", NULL }; addReplyHelp(c, help); From fefde6e3e4addf860829d4848386d90889590eac Mon Sep 17 00:00:00 2001 From: Itamar Haber Date: Sat, 9 Jun 2018 21:03:52 +0300 Subject: [PATCH 4/5] Capitalizes subcommands & orders lexicographically --- src/cluster.c | 40 ++++++++++++++++++++-------------------- src/config.c | 8 ++++---- src/debug.c | 38 +++++++++++++++++++------------------- src/pubsub.c | 6 +++--- src/scripting.c | 10 +++++----- src/server.c | 6 +++--- src/slowlog.c | 6 +++--- 7 files changed, 57 insertions(+), 57 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 56deb927..39aab0cc 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -4178,27 +4178,27 @@ void clusterCommand(client *c) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { const char *help[] = { -"addslots [slot ...] -- Assign slots to current node.", -"bumpepoch -- Advance the cluster config epoch.", -"count-failure-reports -- Return number of failure reports for .", -"countkeysinslot - Return the number of keys in .", -"delslots [slot ...] -- Delete slots information from current node.", -"failover [force|takeover] -- Promote current slave node to being a master.", -"forget -- Remove a node from the cluster.", -"getkeysinslot -- Return key names stored by current node in a slot.", -"flushslots -- Delete current node own slots information.", -"info - Return onformation about the cluster.", -"keyslot -- Return the hash slot for .", -"meet [bus-port] -- Connect nodes into a working cluster.", -"myid -- Return the node id.", -"nodes -- Return cluster configuration seen by node. Output format:", +"ADDSLOTS [slot ...] -- Assign slots to current node.", +"BUMPEPOCH -- Advance the cluster config epoch.", +"COUNT-failure-reports -- Return number of failure reports for .", +"COUNTKEYSINSLOT - Return the number of keys in .", +"DELSLOTS [slot ...] -- Delete slots information from current node.", +"FAILOVER [force|takeover] -- Promote current slave node to being a master.", +"FORGET -- Remove a node from the cluster.", +"GETKEYSINSLOT -- Return key names stored by current node in a slot.", +"FLUSHSLOTS -- Delete current node own slots information.", +"INFO - Return onformation about the cluster.", +"KEYSLOT -- Return the hash slot for .", +"MEET [bus-port] -- Connect nodes into a working cluster.", +"MYID -- Return the node id.", +"NODES -- Return cluster configuration seen by node. Output format:", " ... ", -"replicate -- Configure current node as slave to .", -"reset [hard|soft] -- Reset current node (default: soft).", -"set-config-epoch - Set config epoch of current node.", -"setslot (importing|migrating|stable|node ) -- Set slot state.", -"slaves -- Return slaves.", -"slots -- Return information about slots range mappings. Each range is made of:", +"REPLICATE -- Configure current node as slave to .", +"RESET [hard|soft] -- Reset current node (default: soft).", +"SET-config-epoch - Set config epoch of current node.", +"SETSLOT (importing|migrating|stable|node ) -- Set slot state.", +"SLAVES -- Return slaves.", +"SLOTS -- Return information about slots range mappings. Each range is made of:", " start, end, master and replicas IP addresses, ports and ids", NULL }; diff --git a/src/config.c b/src/config.c index 4e6438e8..edcafa7a 100644 --- a/src/config.c +++ b/src/config.c @@ -2121,10 +2121,10 @@ void configCommand(client *c) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { const char *help[] = { -"get -- Return parameters matching the glob-like and their values.", -"set -- Set parameter to value.", -"resetstat -- Reset statistics reported by INFO.", -"rewrite -- Rewrite the configuration file.", +"GET -- Return parameters matching the glob-like and their values.", +"SET -- Set parameter to value.", +"RESETSTAT -- Reset statistics reported by INFO.", +"REWRITE -- Rewrite the configuration file.", NULL }; addReplyHelp(c, help); diff --git a/src/debug.c b/src/debug.c index 0c5ea9a9..7a9bddeb 100644 --- a/src/debug.c +++ b/src/debug.c @@ -285,25 +285,25 @@ void computeDatasetDigest(unsigned char *final) { void debugCommand(client *c) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { const char *help[] = { -"assert -- Crash by assertion failed.", -"change-repl-id -- Change the replication IDs of the instance. Dangerous, should be used only for testing the replication subsystem.", -"crash-and-recover -- Hard crash and restart after delay.", -"digest -- Outputs an hex signature representing the current DB content.", -"htstats -- Return hash table statistics of the specified Redis database.", -"loadaof -- Flush the AOF buffers on disk and reload the AOF in memory.", -"lua-always-replicate-commands (0|1) -- Setting it to 1 makes Lua replication defaulting to replicating single commands, without the script having to enable effects replication.", -"object -- Show low level info about key and associated value.", -"panic -- Crash the server simulating a panic.", -"populate [prefix] [size] -- Create string keys named key:. If a prefix is specified is used instead of the 'key' prefix.", -"reload -- Save the RDB on disk and reload it back in memory.", -"restart -- Graceful restart: save config, db, restart.", -"sdslen -- Show low level SDS string info representing key and value.", -"segfault -- Crash the server with sigsegv.", -"set-active-expire (0|1) -- Setting it to 0 disables expiring keys in background when they are not accessed (otherwise the Redis behavior). Setting it to 1 reenables back the default.", -"sleep -- Stop the server for . Decimals allowed.", -"structsize -- Return the size of different Redis core C structures.", -"ziplist -- Show low level info about the ziplist encoding.", -"error -- Return a Redis protocol error with as message. Useful for clients unit tests to simulate Redis errors.", +"ASSERT -- Crash by assertion failed.", +"CHANGE-REPL-ID -- Change the replication IDs of the instance. Dangerous, should be used only for testing the replication subsystem.", +"CRASH-and-recover -- Hard crash and restart after delay.", +"DIGEST -- Output a hex signature representing the current DB content.", +"ERROR -- Return a Redis protocol error with as message. Useful for clients unit tests to simulate Redis errors.", +"HTSTATS -- Return hash table statistics of the specified Redis database.", +"LOADAOF -- Flush the AOF buffers on disk and reload the AOF in memory.", +"LUA-ALWAYS-REPLICATE-COMMANDS (0|1) -- Setting it to 1 makes Lua replication defaulting to replicating single commands, without the script having to enable effects replication.", +"OBJECT -- Show low level info about key and associated value.", +"PANIC -- Crash the server simulating a panic.", +"POPULATE [prefix] [size] -- Create string keys named key:. If a prefix is specified is used instead of the 'key' prefix.", +"RELOAD -- Save the RDB on disk and reload it back in memory.", +"RESTART -- Graceful restart: save config, db, restart.", +"SDSLEN -- Show low level SDS string info representing key and value.", +"SEGFAULT -- Crash the server with sigsegv.", +"SET-ACTIVE-EXPIRE (0|1) -- Setting it to 0 disables expiring keys in background when they are not accessed (otherwise the Redis behavior). Setting it to 1 reenables back the default.", +"SLEEP -- Stop the server for . Decimals allowed.", +"STRUCTSIZE -- Return the size of different Redis core C structures.", +"ZIPLIST -- Show low level info about the ziplist encoding.", NULL }; addReplyHelp(c, help); diff --git a/src/pubsub.c b/src/pubsub.c index 7f7be52d..86a7f1c5 100644 --- a/src/pubsub.c +++ b/src/pubsub.c @@ -327,9 +327,9 @@ void publishCommand(client *c) { void pubsubCommand(client *c) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { const char *help[] = { -"channels [] -- Return the currently active channels matching a pattern (default: all).", -"numpat -- Return number of subscriptions to patterns.", -"numsub [channel-1 .. channel-N] -- Returns the number of subscribers for the specified channels (excluding patterns, default: none).", +"CHANNELS [] -- Return the currently active channels matching a pattern (default: all).", +"NUMPAT -- Return number of subscriptions to patterns.", +"NUMSUB [channel-1 .. channel-N] -- Returns the number of subscribers for the specified channels (excluding patterns, default: none).", NULL }; addReplyHelp(c, help); diff --git a/src/scripting.c b/src/scripting.c index 85bce7be..f65540d8 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -1457,11 +1457,11 @@ void evalShaCommand(client *c) { void scriptCommand(client *c) { if (c->argc == 2 && !strcasecmp(c->argv[1]->ptr,"help")) { const char *help[] = { -"debug (yes|sync|no) -- Set the debug mode for subsequent scripts executed.", -"exists [ ...] -- Return information about the existence of the scripts in the script cache.", -"flush -- Flush the Lua scripts cache. Very dangerous on slaves.", -"kill -- Kill the currently executing Lua script.", -"load