multiple cleanups/refactoring, see ChangeLog

git-svn-id: http://code.sixapart.com/svn/memcached/trunk/server@468 b0b603af-a30f-0410-a34e-baf09ae79d0b

ChangeLog

@@ -1,11 +1,27 @@
 2007-03-05  Paul Lindner  <lindner@inuus.com>
+	* Fix a number of places where (s)printf calls were using unsigned
+	  or signed formats that did not match their arguments.
 
+	* Add support for stdbool.h and stdint.h to use the bool and
+	  uint8_t types.
+
+	* Major refactoring - move API calls for assoc/items/slabs to
+	  their own individual header files.  Add apropriate const and
+	  static declarations as appropriate.
+
 	* Avoid type-punning.  Do a more efficient realloc inside the
 	  conn_shrink routine.
 
         * Fix overflow bug where uninitialized access to slabclass caused
 	  size-0 mallocs during slab preallocation.
 
+	* Use EXIT_SUCCESS/EXIT_FAILURE constants.
+
+	* Convert some sprintf calls to snprintf to protect against
+	  buffer overflows.
+
+	* Explicitly compare against NULL or zero in many places.
+
 2006-12-23
 	* fix expirations of items set with absolute expiration times in
 	  the past, before the server's start time.  bug was introduced in

Makefile.am

@@ -1,6 +1,6 @@
 bin_PROGRAMS = memcached memcached-debug
 
-memcached_SOURCES = memcached.c slabs.c items.c assoc.c memcached.h
+memcached_SOURCES = memcached.c slabs.c slabs.h items.c items.h assoc.c assoc.h memcached.h 
 memcached_debug_SOURCES = $(memcached_SOURCES)
 memcached_CPPFLAGS = -DNDEBUG
 memcached_LDADD = @LIBOBJS@

assoc.c

@@ -142,10 +142,10 @@ and these came close:
 }
 
 #if HASH_LITTLE_ENDIAN == 1
-uint32_t hash( 
+static uint32_t hash( 
   const void *key,       /* the key to hash */
   size_t      length,    /* length of the key */
-  uint32_t    initval)   /* initval */
+  const uint32_t    initval)   /* initval */
 {
   uint32_t a,b,c;                                          /* internal state */
   union { const void *ptr; size_t i; } u;     /* needed for Mac Powerbook G4 */
@@ -323,7 +323,7 @@ uint32_t hash(
  * from hashlittle() on all machines.  hashbig() takes advantage of
  * big-endian byte ordering. 
  */
-uint32_t hash( const void *key, size_t length, uint32_t initval)
+static uint32_t hash( const void *key, size_t length, const uint32_t initval)
 {
   uint32_t a,b,c;
   union { const void *ptr; size_t i; } u; /* to cast key to (size_t) happily */
@@ -454,7 +454,7 @@ typedef  unsigned long  int  ub4;   /* unsigned 4-byte quantities */
 typedef  unsigned       char ub1;   /* unsigned 1-byte quantities */
 
 /* how many powers of 2's worth of buckets we use */
-int hashpower = 16;
+static int hashpower = 16;
 
 #define hashsize(n) ((ub4)1<<(n))
 #define hashmask(n) (hashsize(n)-1)
@@ -490,7 +490,7 @@ void assoc_init(void) {
     memset(primary_hashtable, 0, hash_size);
 }
 
-item *assoc_find(const char *key, size_t nkey) {
+item *assoc_find(const char *key, const size_t nkey) {
     uint32_t hv = hash(key, nkey, 0);
     item *it;
     int oldbucket;
@@ -516,7 +516,7 @@ item *assoc_find(const char *key, size_t nkey) {
 /* returns the address of the item pointer before the key.  if *item == 0,
    the item wasn't found */
 
-static item** _hashitem_before (const char *key, size_t nkey) {
+static item** _hashitem_before (const char *key, const size_t nkey) {
     uint32_t hv = hash(key, nkey, 0);
     item **pos;
     int oldbucket;
@@ -603,7 +603,7 @@ int assoc_insert(item *it) {
     return 1;
 }
 
-void assoc_delete(const char *key, size_t nkey) {
+void assoc_delete(const char *key, const size_t nkey) {
     item **before = _hashitem_before(key, nkey);
 
     if (*before) {

assoc.h

@@ -0,0 +1,6 @@
+/* associative array */
+void assoc_init(void);
+item *assoc_find(const char *key, const size_t nkey);
+int assoc_insert(item *item);
+void assoc_delete(const char *key, const size_t nkey);
+void assoc_move_next_bucket(void);

configure.ac

@@ -97,7 +97,8 @@ AC_SEARCH_LIBS(mallinfo, malloc)
 
 AC_CHECK_FUNC(daemon,AC_DEFINE([HAVE_DAEMON],,[Define this if you have daemon()]),[AC_LIBOBJ(daemon)])
 
-
+AC_HEADER_STDBOOL
+AC_C_CONST
 AC_CHECK_HEADER(malloc.h, AC_DEFINE(HAVE_MALLOC_H,,[do we have malloc.h?]))
 AC_CHECK_MEMBER([struct mallinfo.arena], [
 		AC_DEFINE(HAVE_STRUCT_MALLINFO,,[do we have stuct mallinfo?])

items.c

@@ -19,6 +19,10 @@
 
 #include "memcached.h"
 
+/* Forward Declarations */
+static void item_link_q(item *it);
+static void item_unlink_q(item *it);
+
 /*
  * We only reposition items in the LRU queue if they haven't been repositioned
  * in this many seconds. That saves us from churning on frequently-accessed
@@ -29,7 +33,7 @@
 #define LARGEST_ID 255
 static item *heads[LARGEST_ID];
 static item *tails[LARGEST_ID];
-unsigned int sizes[LARGEST_ID];
+static unsigned int sizes[LARGEST_ID];
 
 void item_init(void) {
     int i;
@@ -53,14 +57,17 @@ void item_init(void) {
  *
  * Returns the total size of the header.
  */
-int item_make_header(char *key, uint8_t nkey, int flags, int nbytes,
-                     char *suffix, int *nsuffix) {
-    *nsuffix = sprintf(suffix, " %u %u\r\n", flags, nbytes - 2);
+static size_t item_make_header(char *key, const uint8_t nkey, const int flags, const int nbytes,
+                     char *suffix, uint8_t *nsuffix) {
+    /* suffix is defined at 40 chars elsewhere.. */
+    *nsuffix = (uint8_t) snprintf(suffix, 40, " %d %d\r\n", flags, nbytes - 2);
     return sizeof(item) + nkey + *nsuffix + nbytes;
 }
 
-item *item_alloc(char *key, size_t nkey, int flags, rel_time_t exptime, int nbytes) {
-    int nsuffix, ntotal;
+/*@null@*/
+item *item_alloc(char *key, const size_t nkey, const int flags, const rel_time_t exptime, const int nbytes) {
+    uint8_t nsuffix;
+    size_t ntotal;
     item *it;
     unsigned int id;
     char suffix[40];
@@ -80,7 +87,7 @@ item *item_alloc(char *key, size_t nkey, int flags, rel_time_t exptime, int nbyt
          * we're out of luck at this point...
          */
 
-        if (!settings.evict_to_free) return 0;
+        if (settings.evict_to_free == 0) return NULL;
 
         /* 
          * try to get one off the right LRU 
@@ -89,8 +96,8 @@ item *item_alloc(char *key, size_t nkey, int flags, rel_time_t exptime, int nbyt
          * tries
          */
 
-        if (id > LARGEST_ID) return 0;
-        if (tails[id]==0) return 0;
+        if (id > LARGEST_ID) return NULL;
+        if (tails[id]==0) return NULL;
 
         for (search = tails[id]; tries>0 && search; tries--, search=search->prev) {
             if (search->refcount==0) {
@@ -99,7 +106,7 @@ item *item_alloc(char *key, size_t nkey, int flags, rel_time_t exptime, int nbyt
             }
         }
         it = slabs_alloc(ntotal);
-        if (it==0) return 0;
+        if (it==0) return NULL;
     }
 
     assert(it->slabs_clsid == 0);
@@ -115,13 +122,13 @@ item *item_alloc(char *key, size_t nkey, int flags, rel_time_t exptime, int nbyt
     it->nbytes = nbytes;
     strcpy(ITEM_key(it), key);
     it->exptime = exptime;
-    memcpy(ITEM_suffix(it), suffix, nsuffix);
+    memcpy(ITEM_suffix(it), suffix, (size_t) nsuffix);
     it->nsuffix = nsuffix;
     return it;
 }
 
 void item_free(item *it) {
-    unsigned int ntotal = ITEM_ntotal(it);
+    size_t ntotal = ITEM_ntotal(it);
     assert((it->it_flags & ITEM_LINKED) == 0);
     assert(it != heads[it->slabs_clsid]);
     assert(it != tails[it->slabs_clsid]);
@@ -137,15 +144,15 @@ void item_free(item *it) {
  * Returns true if an item will fit in the cache (its size does not exceed
  * the maximum for a cache entry.)
  */
-int item_size_ok(char *key, size_t nkey, int flags, int nbytes) {
+bool item_size_ok(char *key, const size_t nkey, const int flags, const int nbytes) {
     char prefix[40];
-    int nsuffix;
+    uint8_t nsuffix;
 
     return slabs_clsid(item_make_header(key, nkey + 1, flags, nbytes,
                                         prefix, &nsuffix)) != 0;
 }
 
-void item_link_q(item *it) { /* item is the new head */
+static void item_link_q(item *it) { /* item is the new head */
     item **head, **tail;
     /* always true, warns: assert(it->slabs_clsid <= LARGEST_ID); */
     assert((it->it_flags & ITEM_SLABBED) == 0);
@@ -163,7 +170,7 @@ void item_link_q(item *it) { /* item is the new head */
     return;
 }
 
-void item_unlink_q(item *it) {
+static void item_unlink_q(item *it) {
     item **head, **tail;
     /* always true, warns: assert(it->slabs_clsid <= LARGEST_ID); */
     head = &heads[it->slabs_clsid];
@@ -203,7 +210,7 @@ int item_link(item *it) {
 }
 
 void item_unlink(item *it) {
-    if (it->it_flags & ITEM_LINKED) {
+    if ((it->it_flags & ITEM_LINKED) != 0) {
         it->it_flags &= ~ITEM_LINKED;
         stats.curr_bytes -= ITEM_ntotal(it);
         stats.curr_items -= 1;
@@ -215,8 +222,8 @@ void item_unlink(item *it) {
 
 void item_remove(item *it) {
     assert((it->it_flags & ITEM_SLABBED) == 0);
-    if (it->refcount) it->refcount--;
-    assert((it->it_flags & ITEM_DELETED) == 0 || it->refcount);
+    if (it->refcount != 0) it->refcount--;
+    assert((it->it_flags & ITEM_DELETED) == 0 || it->refcount != 0);
     if (it->refcount == 0 && (it->it_flags & ITEM_LINKED) == 0) {
         item_free(it);
     }
@@ -239,25 +246,25 @@ int item_replace(item *it, item *new_it) {
     return item_link(new_it);
 }
 
-char *item_cachedump(unsigned int slabs_clsid, unsigned int limit, unsigned int *bytes) {
-
+/*@null@*/
+char *item_cachedump(const unsigned int slabs_clsid, const unsigned int limit, unsigned int *bytes) {
     int memlimit = 2*1024*1024;
     char *buffer;
-    int bufcurr;
+    unsigned int bufcurr;
     item *it;
     int len;
     int shown = 0;
     char temp[512];
 
-    if (slabs_clsid > LARGEST_ID) return 0;
+    if (slabs_clsid > LARGEST_ID) return NULL;
     it = heads[slabs_clsid];
 
-    buffer = malloc(memlimit);
-    if (buffer == 0) return 0;
+    buffer = malloc((size_t)memlimit);
+    if (buffer == 0) return NULL;
     bufcurr = 0;
 
-    while (it && (!limit || shown < limit)) {
-        len = sprintf(temp, "ITEM %s [%u b; %lu s]\r\n", ITEM_key(it), it->nbytes - 2, it->time + stats.started);
+    while (it != NULL && (limit==0 || shown < limit)) {
+        len = snprintf(temp, 512, "ITEM %s [%d b; %lu s]\r\n", ITEM_key(it), it->nbytes - 2, it->time + stats.started);
         if (bufcurr + len + 6 > memlimit)  /* 6 is END\r\n\0 */
             break;
         strcpy(buffer + bufcurr, temp);
@@ -273,7 +280,7 @@ char *item_cachedump(unsigned int slabs_clsid, unsigned int limit, unsigned int
     return buffer;
 }
 
-void item_stats(char *buffer, int buflen) {
+void item_stats(char *buffer, const int buflen) {
     int i;
     char *bufcurr = buffer;
     rel_time_t now = current_time;
@@ -285,34 +292,35 @@ void item_stats(char *buffer, int buflen) {
 
     for (i=0; i<LARGEST_ID; i++) {
         if (tails[i])
-            bufcurr += sprintf(bufcurr, "STAT items:%u:number %u\r\nSTAT items:%u:age %u\r\n",
+            bufcurr += snprintf(bufcurr, (size_t)buflen, "STAT items:%d:number %u\r\nSTAT items:%d:age %u\r\n",
                                i, sizes[i], i, now - tails[i]->time);
     }
     strcpy(bufcurr, "END");
     return;
 }
 
 /* dumps out a list of objects of each size, with granularity of 32 bytes */
+/*@null@*/
 char* item_stats_sizes(int *bytes) {
-    int num_buckets = 32768;   /* max 1MB object, divided into 32 bytes size buckets */
-    unsigned int *histogram = (unsigned int*) malloc(num_buckets * sizeof(int));
+    const int num_buckets = 32768;   /* max 1MB object, divided into 32 bytes size buckets */
+    unsigned int *histogram = (unsigned int*) malloc((size_t)num_buckets * sizeof(int));
     char *buf = (char*) malloc(1024*1024*2*sizeof(char));
     int i;
 
     if (histogram == 0 || buf == 0) {
         if (histogram) free(histogram);
         if (buf) free(buf);
-        return 0;
+        return NULL;
     }
 
     /* build the histogram */
-    memset(histogram, 0, num_buckets * sizeof(int));
+    memset(histogram, 0, (size_t) num_buckets * sizeof(int));
     for (i=0; i<LARGEST_ID; i++) {
         item *iter = heads[i];
         while (iter) {
             int ntotal = ITEM_ntotal(iter);
             int bucket = ntotal / 32;
-            if (ntotal % 32) bucket++;
+            if ((ntotal % 32) != 0) bucket++;
             if (bucket < num_buckets) histogram[bucket]++;
             iter = iter->next;
         }
@@ -321,8 +329,8 @@ char* item_stats_sizes(int *bytes) {
     /* write the buffer */
     *bytes = 0;
     for (i=0; i<num_buckets; i++) {
-        if (histogram[i]) {
-            *bytes += sprintf(&buf[*bytes], "%u %u\r\n", i*32, histogram[i]);
+        if (histogram[i] != 0) {
+            *bytes += sprintf(&buf[*bytes], "%d %u\r\n", i*32, histogram[i]);
         }
     }
     *bytes += sprintf(&buf[*bytes], "END\r\n");
@@ -334,7 +342,7 @@ char* item_stats_sizes(int *bytes) {
 void item_flush_expired() {
     int i;
     item *iter, *next;
-    if (! settings.oldest_live)
+    if (settings.oldest_live == 0)
         return;
     for (i = 0; i < LARGEST_ID; i++) {
         /* The LRU is sorted in decreasing time order, and an item's timestamp

items.h

@@ -0,0 +1,20 @@
+/* See items.c */
+void item_init(void);
+/*@null@*/
+item *item_alloc(char *key, const size_t nkey, const int flags, const rel_time_t exptime, const int nbytes);
+void item_free(item *it);
+bool item_size_ok(char *key, const size_t nkey, const int flags, const int nbytes);
+
+int item_link(item *it);    /* may fail if transgresses limits */
+void item_unlink(item *it);
+void item_remove(item *it);
+void item_update(item *it);   /* update LRU time to current and reposition */
+int item_replace(item *it, item *new_it);
+
+/*@null@*/
+char *item_cachedump(const unsigned int slabs_clsid, const unsigned int limit, unsigned int *bytes);
+void item_stats(char *buffer, const int buflen);
+
+/*@null@*/
+char *item_stats_sizes(int *bytes);
+void item_flush_expired(void);