From f704360462640a88975eeb68fd80617921d7c97d Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Sun, 18 Jan 2015 15:54:30 -0500 Subject: [PATCH 1/2] Improve RDB type correctness It's possible large objects could be larger than 'int', so let's upgrade all size counters to ssize_t. This also fixes rdbSaveObject serialized bytes calculation. Since entire serializations of data structures can be large, so we don't want to limit their calculated size to a 32 bit signed max. This commit increases object size calculation and cascades the change back up to serializedlength printing. Before: 127.0.0.1:6379> debug object hihihi ... encoding:quicklist serializedlength:-2147483559 ... After: 127.0.0.1:6379> debug object hihihi ... encoding:quicklist serializedlength:2147483737 ... --- src/debug.c | 4 ++-- src/rdb.c | 26 +++++++++++++------------- src/rdb.h | 5 ++--- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/debug.c b/src/debug.c index 7783196a..e6e7e1e2 100644 --- a/src/debug.c +++ b/src/debug.c @@ -336,10 +336,10 @@ void debugCommand(redisClient *c) { addReplyStatusFormat(c, "Value at:%p refcount:%d " - "encoding:%s serializedlength:%lld " + "encoding:%s serializedlength:%zu " "lru:%d lru_seconds_idle:%llu%s", (void*)val, val->refcount, - strenc, (long long) rdbSavedObjectLen(val), + strenc, rdbSavedObjectLen(val), val->lru, estimateObjectIdleTime(val)/1000, extra); } else if (!strcasecmp(c->argv[1]->ptr,"sdslen") && c->argc == 3) { dictEntry *de; diff --git a/src/rdb.c b/src/rdb.c index 36ba151c..8165ef26 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -222,10 +222,10 @@ int rdbTryIntegerEncoding(char *s, size_t len, unsigned char *enc) { return rdbEncodeInteger(value,enc); } -int rdbSaveLzfBlob(rio *rdb, void *data, size_t compress_len, - size_t original_len) { +ssize_t rdbSaveLzfBlob(rio *rdb, void *data, size_t compress_len, + size_t original_len) { unsigned char byte; - int n, nwritten = 0; + ssize_t n, nwritten = 0; /* Data compressed! Let's save it on disk */ byte = (REDIS_RDB_ENCVAL<<6)|REDIS_RDB_ENC_LZF; @@ -247,7 +247,7 @@ writeerr: return -1; } -int rdbSaveLzfStringObject(rio *rdb, unsigned char *s, size_t len) { +ssize_t rdbSaveLzfStringObject(rio *rdb, unsigned char *s, size_t len) { size_t comprlen, outlen; void *out; @@ -260,7 +260,7 @@ int rdbSaveLzfStringObject(rio *rdb, unsigned char *s, size_t len) { zfree(out); return 0; } - size_t nwritten = rdbSaveLzfBlob(rdb, out, comprlen, len); + ssize_t nwritten = rdbSaveLzfBlob(rdb, out, comprlen, len); zfree(out); return nwritten; } @@ -305,9 +305,9 @@ err: /* Save a string object as [len][data] on disk. If the object is a string * representation of an integer value we try to save it in a special form */ -int rdbSaveRawString(rio *rdb, unsigned char *s, size_t len) { +ssize_t rdbSaveRawString(rio *rdb, unsigned char *s, size_t len) { int enclen; - int n, nwritten = 0; + ssize_t n, nwritten = 0; /* Try integer encoding */ if (len <= 11) { @@ -338,9 +338,9 @@ int rdbSaveRawString(rio *rdb, unsigned char *s, size_t len) { } /* Save a long long value as either an encoded string or a string. */ -int rdbSaveLongLongAsStringObject(rio *rdb, long long value) { +ssize_t rdbSaveLongLongAsStringObject(rio *rdb, long long value) { unsigned char buf[32]; - int n, nwritten = 0; + ssize_t n, nwritten = 0; int enclen = rdbEncodeInteger(value,buf); if (enclen > 0) { return rdbWriteRaw(rdb,buf,enclen); @@ -532,8 +532,8 @@ int rdbLoadObjectType(rio *rdb) { } /* Save a Redis object. Returns -1 on error, number of bytes written on success. */ -int rdbSaveObject(rio *rdb, robj *o) { - int n = 0, nwritten = 0; +ssize_t rdbSaveObject(rio *rdb, robj *o) { + ssize_t n = 0, nwritten = 0; if (o->type == REDIS_STRING) { /* Save a string value */ @@ -654,8 +654,8 @@ int rdbSaveObject(rio *rdb, robj *o) { * the rdbSaveObject() function. Currently we use a trick to get * this length with very little changes to the code. In the future * we could switch to a faster solution. */ -off_t rdbSavedObjectLen(robj *o) { - int len = rdbSaveObject(NULL,o); +size_t rdbSavedObjectLen(robj *o) { + ssize_t len = rdbSaveObject(NULL,o); redisAssertWithInfo(NULL,o,len != -1); return len; } diff --git a/src/rdb.h b/src/rdb.h index 6319f5d0..a72607b7 100644 --- a/src/rdb.h +++ b/src/rdb.h @@ -109,9 +109,8 @@ int rdbSaveBackground(char *filename); int rdbSaveToSlavesSockets(void); void rdbRemoveTempFile(pid_t childpid); int rdbSave(char *filename); -int rdbSaveObject(rio *rdb, robj *o); -off_t rdbSavedObjectLen(robj *o); -off_t rdbSavedObjectPages(robj *o); +ssize_t rdbSaveObject(rio *rdb, robj *o); +size_t rdbSavedObjectLen(robj *o); robj *rdbLoadObject(int type, rio *rdb); void backgroundSaveDoneHandler(int exitcode, int bysignal); int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, long long expiretime, long long now); From 53c082ec39fb4daafba09e416279265f20d46006 Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Sun, 18 Jan 2015 16:46:25 -0500 Subject: [PATCH 2/2] Improve networking type correctness read() and write() return ssize_t (signed long), not int. For other offsets, we can use the unsigned size_t type instead of a signed offset (since our replication offsets and buffer positions are never negative). --- src/anet.c | 4 ++-- src/cluster.c | 2 +- src/networking.c | 5 +++-- src/redis-benchmark.c | 4 ++-- src/redis.h | 10 +++++----- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/anet.c b/src/anet.c index 76e9b67a..0ec5c55a 100644 --- a/src/anet.c +++ b/src/anet.c @@ -391,7 +391,7 @@ int anetUnixNonBlockConnect(char *err, char *path) * (unless error or EOF condition is encountered) */ int anetRead(int fd, char *buf, int count) { - int nread, totlen = 0; + ssize_t nread, totlen = 0; while(totlen != count) { nread = read(fd,buf,count-totlen); if (nread == 0) return totlen; @@ -406,7 +406,7 @@ int anetRead(int fd, char *buf, int count) * (unless error is encountered) */ int anetWrite(int fd, char *buf, int count) { - int nwritten, totlen = 0; + ssize_t nwritten, totlen = 0; while(totlen != count) { nwritten = write(fd,buf,count-totlen); if (nwritten == 0) return totlen; diff --git a/src/cluster.c b/src/cluster.c index ec6901e8..826c7f41 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -4462,7 +4462,7 @@ try_again: { sds buf = cmd.io.buffer.ptr; size_t pos = 0, towrite; - int nwritten = 0; + ssize_t nwritten = 0; while ((towrite = sdslen(buf)-pos) > 0) { towrite = (towrite > (64*1024) ? (64*1024) : towrite); diff --git a/src/networking.c b/src/networking.c index 607d225f..0b69f540 100644 --- a/src/networking.c +++ b/src/networking.c @@ -797,7 +797,8 @@ void freeClientsInAsyncFreeQueue(void) { void sendReplyToClient(aeEventLoop *el, int fd, void *privdata, int mask) { redisClient *c = privdata; - int nwritten = 0, totwritten = 0, objlen; + ssize_t nwritten = 0, totwritten = 0; + size_t objlen; size_t objmem; robj *o; REDIS_NOTUSED(el); @@ -1621,7 +1622,7 @@ int checkClientOutputBufferLimits(redisClient *c) { * called from contexts where the client can't be freed safely, i.e. from the * lower level functions pushing data inside the client output buffers. */ void asyncCloseClientOnOutputBufferLimitReached(redisClient *c) { - redisAssert(c->reply_bytes < ULONG_MAX-(1024*64)); + redisAssert(c->reply_bytes < SIZE_MAX-(1024*64)); if (c->reply_bytes == 0 || c->flags & REDIS_CLOSE_ASAP) return; if (checkClientOutputBufferLimits(c)) { sds client = catClientInfoString(sdsempty(),c); diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 7567e018..f735aeb6 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -86,7 +86,7 @@ typedef struct _client { char **randptr; /* Pointers to :rand: strings inside the command buf */ size_t randlen; /* Number of pointers in client->randptr */ size_t randfree; /* Number of unused pointers in client->randptr */ - unsigned int written; /* Bytes of 'obuf' already written */ + size_t written; /* Bytes of 'obuf' already written */ long long start; /* Start time of a request */ long long latency; /* Request latency */ int pending; /* Number of pending requests (replies to consume) */ @@ -266,7 +266,7 @@ static void writeHandler(aeEventLoop *el, int fd, void *privdata, int mask) { if (sdslen(c->obuf) > c->written) { void *ptr = c->obuf+c->written; - int nwritten = write(c->context->fd,ptr,sdslen(c->obuf)-c->written); + ssize_t nwritten = write(c->context->fd,ptr,sdslen(c->obuf)-c->written); if (nwritten == -1) { if (errno != EPIPE) fprintf(stderr, "Writing to socket: %s\n", strerror(errno)); diff --git a/src/redis.h b/src/redis.h index 0c191d06..6a8308f7 100644 --- a/src/redis.h +++ b/src/redis.h @@ -542,8 +542,8 @@ typedef struct redisClient { int multibulklen; /* number of multi bulk arguments left to read */ long bulklen; /* length of bulk argument in multi bulk request */ list *reply; - unsigned long reply_bytes; /* Tot bytes of objects in reply list */ - int sentlen; /* Amount of bytes already sent in the current + size_t reply_bytes; /* Tot bytes of objects in reply list */ + size_t sentlen; /* Amount of bytes already sent in the current buffer or object being sent. */ time_t ctime; /* Client creation time */ time_t lastinteraction; /* time of the last interaction, used for timeout */ @@ -553,8 +553,8 @@ typedef struct redisClient { int replstate; /* replication state if this is a slave */ int repl_put_online_on_ack; /* Install slave write handler on ACK. */ int repldbfd; /* replication DB file descriptor */ - off_t repldboff; /* replication DB file offset */ - off_t repldbsize; /* replication DB file size */ + size_t repldboff; /* replication DB file offset */ + size_t repldbsize; /* replication DB file size */ sds replpreamble; /* replication DB preamble. */ long long reploff; /* replication offset if this is our master */ long long repl_ack_off; /* replication ack offset, if this is a slave */ @@ -571,7 +571,7 @@ typedef struct redisClient { sds peerid; /* Cached peer ID. */ /* Response buffer */ - int bufpos; + size_t bufpos; char buf[REDIS_REPLY_CHUNK_BYTES]; } redisClient;