Use specialized function to add status and error replies

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";
+        addReplyStatus(c,"Background append only file rewriting started");
-        addReplySds(c,sdsnew(status));
     } else {
         addReply(c,shared.err);
     }

src/config.c

@@ -270,8 +270,8 @@ void configSetCommand(redisClient *c) {
                 stopAppendOnly();
             } else {
                 if (startAppendOnly() == REDIS_ERR) {
-                    addReplySds(c,sdscatprintf(sdsempty(),
+                    addReplyError(c,
-                        "-ERR Unable to turn on AOF. Check server logs.\r\n"));
+                        "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(),
+        addReplyErrorFormat(c,"Unsupported CONFIG parameter: %s",
-            "-ERR not supported CONFIG parameter %s\r\n",
+            (char*)c->argv[2]->ptr);
-            (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(),
+    addReplyErrorFormat(c,"Invalid argument '%s' for CONFIG SET '%s'",
-        "-ERR invalid argument '%s' for CONFIG SET '%s'\r\n",
             (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(),
+        addReplyError(c,
-            "-ERR CONFIG subcommand must be one of GET, SET, RESETSTAT\r\n"));
+            "CONFIG subcommand must be one of GET, SET, RESETSTAT");
     }
     return;
 
 badarity:
-    addReplySds(c,sdscatprintf(sdsempty(),
+    addReplyErrorFormat(c,"Wrong number of arguments for CONFIG %s",
-        "-ERR Wrong number of arguments for CONFIG %s\r\n",
+        (char*) c->argv[1]->ptr);
-        (char*) c->argv[1]->ptr));
 }

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_STRING: type = "string"; break;
-        case REDIS_LIST: type = "+list"; break;
+        case REDIS_LIST: type = "list"; break;
-        case REDIS_SET: type = "+set"; break;
+        case REDIS_SET: type = "set"; break;
-        case REDIS_ZSET: type = "+zset"; break;
+        case REDIS_ZSET: type = "zset"; break;
-        case REDIS_HASH: type = "+hash"; break;
+        case REDIS_HASH: type = "hash"; break;
-        default: type = "+unknown"; break;
+        default: type = "unknown"; break;
         }
     }
-    addReplySds(c,sdsnew(type));
+    addReplyStatus(c,type);
-    addReply(c,shared.crlf);
 }
 
 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";
+        addReplyStatus(c,"Background saving started");
-        addReplySds(c,sdsnew(status));
     } 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) {

src/debug.c

@@ -211,18 +211,18 @@ void debugCommand(redisClient *c) {
             char *strenc;
 
             strenc = strEncoding(val->encoding);
-            addReplySds(c,sdscatprintf(sdsempty(),
+            addReplyStatusFormat(c,
-                "+Value at:%p refcount:%d "
+                "Value at:%p refcount:%d "
-                "encoding:%s serializedlength:%lld\r\n",
+                "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(),
+            addReplyStatusFormat(c,
-                "+Value swapped at: page %llu "
+                "Value swapped at: page %llu "
-                "using %llu pages\r\n",
+                "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]);
-
+        addReplyStatus(c,d);
-        d = sdscatlen(d,"\r\n",2);
+        sdsfree(d);
-        addReplySds(c,d);
     } else {
-        addReplySds(c,sdsnew(
+        addReplyError(c,
-            "-ERR Syntax error, try DEBUG [SEGFAULT|OBJECT <key>|SWAPIN <key>|SWAPOUT <key>|RELOAD]\r\n"));
+            "Syntax error, try DEBUG [SEGFAULT|OBJECT <key>|SWAPIN <key>|SWAPOUT <key>|RELOAD]");
     }
 }
 

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++)

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) {

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;
     }

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,
+        addReplyErrorFormat(c,"unknown command '%s'",
-            sdscatprintf(sdsempty(), "-ERR unknown command '%s'\r\n",
+            (char*)c->argv[0]->ptr);
-                (char*)c->argv[0]->ptr));
         resetClient(c);
         return 1;
     } else if ((cmd->arity > 0 && cmd->arity != c->argc) ||
                (c->argc < -cmd->arity)) {
-        addReplySds(c,
+        addReplyErrorFormat(c,"wrong number of arguments for '%s' command",
-            sdscatprintf(sdsempty(),
+            cmd->name);
-                "-ERR wrong number of arguments for '%s' command\r\n",
-                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");
     }
 }
 

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);

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;

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;
     }
 

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

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,
+            addReplyError(c,"resulting score is not a number (NaN)");
-                sdsnew("-ERR resulting score is not a number (NaN)\r\n"));
             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,
+        addReplyError(c,"wrong number of arguments for ZRANGEBYSCORE");
-            sdsnew("-ERR wrong number of arguments for ZRANGEBYSCORE\r\n"));
         return;
     }