From 3ab203762f28ffec4036dc4f5a188d637cf78ff1 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 2 Sep 2010 19:52:24 +0200 Subject: [PATCH] Use specialized function to add status and error replies --- src/aof.c | 5 ++--- src/config.c | 23 ++++++++++------------- src/db.c | 28 +++++++++++++--------------- src/debug.c | 33 ++++++++++++++++----------------- src/multi.c | 8 ++++---- src/networking.c | 38 ++++++++++++++++++++++++++++++++++++++ src/object.c | 12 ++++++------ src/redis.c | 25 +++++++++++-------------- src/redis.h | 12 ++++++++++++ src/replication.c | 6 +++--- src/t_hash.c | 2 +- src/t_string.c | 4 ++-- src/t_zset.c | 9 ++++----- 13 files changed, 122 insertions(+), 83 deletions(-) diff --git a/src/aof.c b/src/aof.c index 58dd5538..b639eb52 100644 --- a/src/aof.c +++ b/src/aof.c @@ -632,12 +632,11 @@ int rewriteAppendOnlyFileBackground(void) { void bgrewriteaofCommand(redisClient *c) { if (server.bgrewritechildpid != -1) { - addReplySds(c,sdsnew("-ERR background append only file rewriting already in progress\r\n")); + addReplyError(c,"Background append only file rewriting already in progress"); return; } if (rewriteAppendOnlyFileBackground() == REDIS_OK) { - char *status = "+Background append only file rewriting started\r\n"; - addReplySds(c,sdsnew(status)); + addReplyStatus(c,"Background append only file rewriting started"); } else { addReply(c,shared.err); } diff --git a/src/config.c b/src/config.c index 5c449886..8a5ad6c2 100644 --- a/src/config.c +++ b/src/config.c @@ -270,8 +270,8 @@ void configSetCommand(redisClient *c) { stopAppendOnly(); } else { if (startAppendOnly() == REDIS_ERR) { - addReplySds(c,sdscatprintf(sdsempty(), - "-ERR Unable to turn on AOF. Check server logs.\r\n")); + addReplyError(c, + "Unable to turn on AOF. Check server logs."); decrRefCount(o); return; } @@ -312,9 +312,8 @@ void configSetCommand(redisClient *c) { } sdsfreesplitres(v,vlen); } else { - addReplySds(c,sdscatprintf(sdsempty(), - "-ERR not supported CONFIG parameter %s\r\n", - (char*)c->argv[2]->ptr)); + addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s", + (char*)c->argv[2]->ptr); decrRefCount(o); return; } @@ -323,10 +322,9 @@ void configSetCommand(redisClient *c) { return; badfmt: /* Bad format errors */ - addReplySds(c,sdscatprintf(sdsempty(), - "-ERR invalid argument '%s' for CONFIG SET '%s'\r\n", + addReplyErrorFormat(c,"Invalid argument '%s' for CONFIG SET '%s'", (char*)o->ptr, - (char*)c->argv[2]->ptr)); + (char*)c->argv[2]->ptr); decrRefCount(o); } @@ -425,13 +423,12 @@ void configCommand(redisClient *c) { server.stat_starttime = time(NULL); addReply(c,shared.ok); } else { - addReplySds(c,sdscatprintf(sdsempty(), - "-ERR CONFIG subcommand must be one of GET, SET, RESETSTAT\r\n")); + addReplyError(c, + "CONFIG subcommand must be one of GET, SET, RESETSTAT"); } return; badarity: - addReplySds(c,sdscatprintf(sdsempty(), - "-ERR Wrong number of arguments for CONFIG %s\r\n", - (char*) c->argv[1]->ptr)); + addReplyErrorFormat(c,"Wrong number of arguments for CONFIG %s", + (char*) c->argv[1]->ptr); } diff --git a/src/db.c b/src/db.c index 4d119cf2..4f1572a6 100644 --- a/src/db.c +++ b/src/db.c @@ -199,7 +199,7 @@ void selectCommand(redisClient *c) { int id = atoi(c->argv[1]->ptr); if (selectDb(c,id) == REDIS_ERR) { - addReplySds(c,sdsnew("-ERR invalid DB index\r\n")); + addReplyError(c,"invalid DB index"); } else { addReply(c,shared.ok); } @@ -258,24 +258,23 @@ void typeCommand(redisClient *c) { o = lookupKeyRead(c->db,c->argv[1]); if (o == NULL) { - type = "+none"; + type = "none"; } else { switch(o->type) { - case REDIS_STRING: type = "+string"; break; - case REDIS_LIST: type = "+list"; break; - case REDIS_SET: type = "+set"; break; - case REDIS_ZSET: type = "+zset"; break; - case REDIS_HASH: type = "+hash"; break; - default: type = "+unknown"; break; + case REDIS_STRING: type = "string"; break; + case REDIS_LIST: type = "list"; break; + case REDIS_SET: type = "set"; break; + case REDIS_ZSET: type = "zset"; break; + case REDIS_HASH: type = "hash"; break; + default: type = "unknown"; break; } } - addReplySds(c,sdsnew(type)); - addReply(c,shared.crlf); + addReplyStatus(c,type); } void saveCommand(redisClient *c) { if (server.bgsavechildpid != -1) { - addReplySds(c,sdsnew("-ERR background save in progress\r\n")); + addReplyError(c,"Background save already in progress"); return; } if (rdbSave(server.dbfilename) == REDIS_OK) { @@ -287,12 +286,11 @@ void saveCommand(redisClient *c) { void bgsaveCommand(redisClient *c) { if (server.bgsavechildpid != -1) { - addReplySds(c,sdsnew("-ERR background save already in progress\r\n")); + addReplyError(c,"Background save already in progress"); return; } if (rdbSaveBackground(server.dbfilename) == REDIS_OK) { - char *status = "+Background saving started\r\n"; - addReplySds(c,sdsnew(status)); + addReplyStatus(c,"Background saving started"); } else { addReply(c,shared.err); } @@ -301,7 +299,7 @@ void bgsaveCommand(redisClient *c) { void shutdownCommand(redisClient *c) { if (prepareForShutdown() == REDIS_OK) exit(0); - addReplySds(c, sdsnew("-ERR Errors trying to SHUTDOWN. Check logs.\r\n")); + addReplyError(c,"Errors trying to SHUTDOWN. Check logs."); } void renameGenericCommand(redisClient *c, int nx) { diff --git a/src/debug.c b/src/debug.c index 76d18b21..2f7ab58f 100644 --- a/src/debug.c +++ b/src/debug.c @@ -211,18 +211,18 @@ void debugCommand(redisClient *c) { char *strenc; strenc = strEncoding(val->encoding); - addReplySds(c,sdscatprintf(sdsempty(), - "+Value at:%p refcount:%d " - "encoding:%s serializedlength:%lld\r\n", + addReplyStatusFormat(c, + "Value at:%p refcount:%d " + "encoding:%s serializedlength:%lld", (void*)val, val->refcount, - strenc, (long long) rdbSavedObjectLen(val,NULL))); + strenc, (long long) rdbSavedObjectLen(val,NULL)); } else { vmpointer *vp = (vmpointer*) val; - addReplySds(c,sdscatprintf(sdsempty(), - "+Value swapped at: page %llu " - "using %llu pages\r\n", + addReplyStatusFormat(c, + "Value swapped at: page %llu " + "using %llu pages", (unsigned long long) vp->page, - (unsigned long long) vp->usedpages)); + (unsigned long long) vp->usedpages); } } else if (!strcasecmp(c->argv[1]->ptr,"swapin") && c->argc == 3) { lookupKeyRead(c->db,c->argv[2]); @@ -233,7 +233,7 @@ void debugCommand(redisClient *c) { vmpointer *vp; if (!server.vm_enabled) { - addReplySds(c,sdsnew("-ERR Virtual Memory is disabled\r\n")); + addReplyError(c,"Virtual Memory is disabled"); return; } if (!de) { @@ -243,9 +243,9 @@ void debugCommand(redisClient *c) { val = dictGetEntryVal(de); /* Swap it */ if (val->storage != REDIS_VM_MEMORY) { - addReplySds(c,sdsnew("-ERR This key is not in memory\r\n")); + addReplyError(c,"This key is not in memory"); } else if (val->refcount != 1) { - addReplySds(c,sdsnew("-ERR Object is shared\r\n")); + addReplyError(c,"Object is shared"); } else if ((vp = vmSwapObjectBlocking(val)) != NULL) { dictGetEntryVal(de) = vp; addReply(c,shared.ok); @@ -274,18 +274,17 @@ void debugCommand(redisClient *c) { addReply(c,shared.ok); } else if (!strcasecmp(c->argv[1]->ptr,"digest") && c->argc == 2) { unsigned char digest[20]; - sds d = sdsnew("+"); + sds d = sdsempty(); int j; computeDatasetDigest(digest); for (j = 0; j < 20; j++) d = sdscatprintf(d, "%02x",digest[j]); - - d = sdscatlen(d,"\r\n",2); - addReplySds(c,d); + addReplyStatus(c,d); + sdsfree(d); } else { - addReplySds(c,sdsnew( - "-ERR Syntax error, try DEBUG [SEGFAULT|OBJECT |SWAPIN |SWAPOUT |RELOAD]\r\n")); + addReplyError(c, + "Syntax error, try DEBUG [SEGFAULT|OBJECT |SWAPIN |SWAPOUT |RELOAD]"); } } diff --git a/src/multi.c b/src/multi.c index c85516df..47615eb0 100644 --- a/src/multi.c +++ b/src/multi.c @@ -42,7 +42,7 @@ void queueMultiCommand(redisClient *c, struct redisCommand *cmd) { void multiCommand(redisClient *c) { if (c->flags & REDIS_MULTI) { - addReplySds(c,sdsnew("-ERR MULTI calls can not be nested\r\n")); + addReplyError(c,"MULTI calls can not be nested"); return; } c->flags |= REDIS_MULTI; @@ -51,7 +51,7 @@ void multiCommand(redisClient *c) { void discardCommand(redisClient *c) { if (!(c->flags & REDIS_MULTI)) { - addReplySds(c,sdsnew("-ERR DISCARD without MULTI\r\n")); + addReplyError(c,"DISCARD without MULTI"); return; } @@ -82,7 +82,7 @@ void execCommand(redisClient *c) { int orig_argc; if (!(c->flags & REDIS_MULTI)) { - addReplySds(c,sdsnew("-ERR EXEC without MULTI\r\n")); + addReplyError(c,"EXEC without MULTI"); return; } @@ -251,7 +251,7 @@ void watchCommand(redisClient *c) { int j; if (c->flags & REDIS_MULTI) { - addReplySds(c,sdsnew("-ERR WATCH inside MULTI is not allowed\r\n")); + addReplyError(c,"WATCH inside MULTI is not allowed"); return; } for (j = 1; j < c->argc; j++) diff --git a/src/networking.c b/src/networking.c index dd005335..d62456a3 100644 --- a/src/networking.c +++ b/src/networking.c @@ -194,6 +194,44 @@ void addReplyString(redisClient *c, char *s, size_t len) { _addReplyStringToList(c,s,len); } +void _addReplyError(redisClient *c, char *s, size_t len) { + addReplyString(c,"-ERR ",5); + addReplyString(c,s,len); + addReplyString(c,"\r\n",2); +} + +void addReplyError(redisClient *c, char *err) { + _addReplyError(c,err,strlen(err)); +} + +void addReplyErrorFormat(redisClient *c, const char *fmt, ...) { + va_list ap; + va_start(ap,fmt); + sds s = sdscatvprintf(sdsempty(),fmt,ap); + va_end(ap); + _addReplyError(c,s,sdslen(s)); + sdsfree(s); +} + +void _addReplyStatus(redisClient *c, char *s, size_t len) { + addReplyString(c,"+",1); + addReplyString(c,s,len); + addReplyString(c,"\r\n",2); +} + +void addReplyStatus(redisClient *c, char *status) { + _addReplyStatus(c,status,strlen(status)); +} + +void addReplyStatusFormat(redisClient *c, const char *fmt, ...) { + va_list ap; + va_start(ap,fmt); + sds s = sdscatvprintf(sdsempty(),fmt,ap); + va_end(ap); + _addReplyStatus(c,s,sdslen(s)); + sdsfree(s); +} + /* Adds an empty object to the reply list that will contain the multi bulk * length, which is not known when this function is called. */ void *addDeferredMultiBulkLength(redisClient *c) { diff --git a/src/object.c b/src/object.c index 92af1d6a..c1a08245 100644 --- a/src/object.c +++ b/src/object.c @@ -354,9 +354,9 @@ int getDoubleFromObjectOrReply(redisClient *c, robj *o, double *target, const ch double value; if (getDoubleFromObject(o, &value) != REDIS_OK) { if (msg != NULL) { - addReplySds(c, sdscatprintf(sdsempty(), "-ERR %s\r\n", msg)); + addReplyError(c,(char*)msg); } else { - addReplySds(c, sdsnew("-ERR value is not a double\r\n")); + addReplyError(c,"value is not a double"); } return REDIS_ERR; } @@ -393,9 +393,9 @@ int getLongLongFromObjectOrReply(redisClient *c, robj *o, long long *target, con long long value; if (getLongLongFromObject(o, &value) != REDIS_OK) { if (msg != NULL) { - addReplySds(c, sdscatprintf(sdsempty(), "-ERR %s\r\n", msg)); + addReplyError(c,(char*)msg); } else { - addReplySds(c, sdsnew("-ERR value is not an integer or out of range\r\n")); + addReplyError(c,"value is not an integer or out of range"); } return REDIS_ERR; } @@ -410,9 +410,9 @@ int getLongFromObjectOrReply(redisClient *c, robj *o, long *target, const char * if (getLongLongFromObjectOrReply(c, o, &value, msg) != REDIS_OK) return REDIS_ERR; if (value < LONG_MIN || value > LONG_MAX) { if (msg != NULL) { - addReplySds(c, sdscatprintf(sdsempty(), "-ERR %s\r\n", msg)); + addReplyError(c,(char*)msg); } else { - addReplySds(c, sdsnew("-ERR value is out of range\r\n")); + addReplyError(c,"value is out of range"); } return REDIS_ERR; } diff --git a/src/redis.c b/src/redis.c index 77e67c58..5af9b235 100644 --- a/src/redis.c +++ b/src/redis.c @@ -909,7 +909,7 @@ int processCommand(redisClient *c) { } else if (c->multibulk) { if (c->bulklen == -1) { if (((char*)c->argv[0]->ptr)[0] != '$') { - addReplySds(c,sdsnew("-ERR multi bulk protocol error\r\n")); + addReplyError(c,"multi bulk protocol error"); resetClient(c); return 1; } else { @@ -922,7 +922,7 @@ int processCommand(redisClient *c) { bulklen < 0 || bulklen > 1024*1024*1024) { c->argc--; - addReplySds(c,sdsnew("-ERR invalid bulk write count\r\n")); + addReplyError(c,"invalid bulk write count"); resetClient(c); return 1; } @@ -975,17 +975,14 @@ int processCommand(redisClient *c) { * such wrong arity, bad command name and so forth. */ cmd = lookupCommand(c->argv[0]->ptr); if (!cmd) { - addReplySds(c, - sdscatprintf(sdsempty(), "-ERR unknown command '%s'\r\n", - (char*)c->argv[0]->ptr)); + addReplyErrorFormat(c,"unknown command '%s'", + (char*)c->argv[0]->ptr); resetClient(c); return 1; } else if ((cmd->arity > 0 && cmd->arity != c->argc) || (c->argc < -cmd->arity)) { - addReplySds(c, - sdscatprintf(sdsempty(), - "-ERR wrong number of arguments for '%s' command\r\n", - cmd->name)); + addReplyErrorFormat(c,"wrong number of arguments for '%s' command", + cmd->name); resetClient(c); return 1; } else if (cmd->flags & REDIS_CMD_BULK && c->bulklen == -1) { @@ -999,7 +996,7 @@ int processCommand(redisClient *c) { bulklen < 0 || bulklen > 1024*1024*1024) { c->argc--; - addReplySds(c,sdsnew("-ERR invalid bulk write count\r\n")); + addReplyError(c,"invalid bulk write count"); resetClient(c); return 1; } @@ -1026,7 +1023,7 @@ int processCommand(redisClient *c) { /* Check if the user is authenticated */ if (server.requirepass && !c->authenticated && cmd->proc != authCommand) { - addReplySds(c,sdsnew("-ERR operation not permitted\r\n")); + addReplyError(c,"operation not permitted"); resetClient(c); return 1; } @@ -1035,7 +1032,7 @@ int processCommand(redisClient *c) { if (server.maxmemory && (cmd->flags & REDIS_CMD_DENYOOM) && zmalloc_used_memory() > server.maxmemory) { - addReplySds(c,sdsnew("-ERR command not allowed when used memory > 'maxmemory'\r\n")); + addReplyError(c,"command not allowed when used memory > 'maxmemory'"); resetClient(c); return 1; } @@ -1045,7 +1042,7 @@ int processCommand(redisClient *c) { && cmd->proc != subscribeCommand && cmd->proc != unsubscribeCommand && cmd->proc != psubscribeCommand && cmd->proc != punsubscribeCommand) { - addReplySds(c,sdsnew("-ERR only (P)SUBSCRIBE / (P)UNSUBSCRIBE / QUIT allowed in this context\r\n")); + addReplyError(c,"only (P)SUBSCRIBE / (P)UNSUBSCRIBE / QUIT allowed in this context"); resetClient(c); return 1; } @@ -1109,7 +1106,7 @@ void authCommand(redisClient *c) { addReply(c,shared.ok); } else { c->authenticated = 0; - addReplySds(c,sdscatprintf(sdsempty(),"-ERR invalid password\r\n")); + addReplyError(c,"invalid password"); } } diff --git a/src/redis.h b/src/redis.h index 328df08d..1ef56288 100644 --- a/src/redis.h +++ b/src/redis.h @@ -605,11 +605,23 @@ void addReplyBulkCString(redisClient *c, char *s); void acceptHandler(aeEventLoop *el, int fd, void *privdata, int mask); void addReply(redisClient *c, robj *obj); void addReplySds(redisClient *c, sds s); +void addReplyError(redisClient *c, char *err); +void addReplyStatus(redisClient *c, char *status); void addReplyDouble(redisClient *c, double d); void addReplyLongLong(redisClient *c, long long ll); void addReplyMultiBulkLen(redisClient *c, long length); void *dupClientReplyValue(void *o); +#ifdef __GNUC__ +void addReplyErrorFormat(redisClient *c, const char *fmt, ...) + __attribute__((format(printf, 2, 3))); +void addReplyStatusFormat(redisClient *c, const char *fmt, ...) + __attribute__((format(printf, 2, 3))); +#else +void addReplyErrorFormat(redisClient *c, const char *fmt, ...); +void addReplyStatusFormat(redisClient *c, const char *fmt, ...); +#endif + /* List data type */ void listTypeTryConversion(robj *subject, robj *value); void listTypePush(robj *subject, robj *value, int where); diff --git a/src/replication.c b/src/replication.c index c2846088..8c629006 100644 --- a/src/replication.c +++ b/src/replication.c @@ -179,7 +179,7 @@ void syncCommand(redisClient *c) { /* Refuse SYNC requests if we are a slave but the link with our master * is not ok... */ if (server.masterhost && server.replstate != REDIS_REPL_CONNECTED) { - addReplySds(c,sdsnew("-ERR Can't SYNC while not connected with my master\r\n")); + addReplyError(c,"Can't SYNC while not connected with my master"); return; } @@ -188,7 +188,7 @@ void syncCommand(redisClient *c) { * buffer registering the differences between the BGSAVE and the current * dataset, so that we can copy to other slaves if needed. */ if (listLength(c->reply) != 0) { - addReplySds(c,sdsnew("-ERR SYNC is invalid with pending input\r\n")); + addReplyError(c,"SYNC is invalid with pending input"); return; } @@ -226,7 +226,7 @@ void syncCommand(redisClient *c) { redisLog(REDIS_NOTICE,"Starting BGSAVE for SYNC"); if (rdbSaveBackground(server.dbfilename) != REDIS_OK) { redisLog(REDIS_NOTICE,"Replication failed, can't BGSAVE"); - addReplySds(c,sdsnew("-ERR Unalbe to perform background save\r\n")); + addReplyError(c,"Unable to perform background save"); return; } c->replstate = REDIS_REPL_WAIT_BGSAVE_END; diff --git a/src/t_hash.c b/src/t_hash.c index 5745f88c..5cef1cab 100644 --- a/src/t_hash.c +++ b/src/t_hash.c @@ -249,7 +249,7 @@ void hmsetCommand(redisClient *c) { robj *o; if ((c->argc % 2) == 1) { - addReplySds(c,sdsnew("-ERR wrong number of arguments for HMSET\r\n")); + addReplyError(c,"wrong number of arguments for HMSET"); return; } diff --git a/src/t_string.c b/src/t_string.c index 276f4dab..509c630a 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -12,7 +12,7 @@ void setGenericCommand(redisClient *c, int nx, robj *key, robj *val, robj *expir if (getLongFromObjectOrReply(c, expire, &seconds, NULL) != REDIS_OK) return; if (seconds <= 0) { - addReplySds(c,sdsnew("-ERR invalid expire time in SETEX\r\n")); + addReplyError(c,"invalid expire time in SETEX"); return; } } @@ -98,7 +98,7 @@ void msetGenericCommand(redisClient *c, int nx) { int j, busykeys = 0; if ((c->argc % 2) == 0) { - addReplySds(c,sdsnew("-ERR wrong number of arguments for MSET\r\n")); + addReplyError(c,"wrong number of arguments for MSET"); return; } /* Handle the NX flag. The MSETNX semantic is to return zero and don't diff --git a/src/t_zset.c b/src/t_zset.c index 6a332c6a..d944e923 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -355,8 +355,7 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i *score = scoreval; } if (isnan(*score)) { - addReplySds(c, - sdsnew("-ERR resulting score is not a number (NaN)\r\n")); + addReplyError(c,"resulting score is not a number (NaN)"); zfree(score); /* Note that we don't need to check if the zset may be empty and * should be removed here, as we can only obtain Nan as score if @@ -561,7 +560,8 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { /* expect setnum input keys to be given */ setnum = atoi(c->argv[2]->ptr); if (setnum < 1) { - addReplySds(c,sdsnew("-ERR at least 1 input key is needed for ZUNIONSTORE/ZINTERSTORE\r\n")); + addReplyError(c, + "at least 1 input key is needed for ZUNIONSTORE/ZINTERSTORE"); return; } @@ -839,8 +839,7 @@ void genericZrangebyscoreCommand(redisClient *c, int justcount) { if (c->argc != (4 + withscores) && c->argc != (7 + withscores)) badsyntax = 1; if (badsyntax) { - addReplySds(c, - sdsnew("-ERR wrong number of arguments for ZRANGEBYSCORE\r\n")); + addReplyError(c,"wrong number of arguments for ZRANGEBYSCORE"); return; } -- 2.45.2