Fix alignment, unsigned/signed, and size issues.

4 files changed, 176 insertions, 129 deletions

diff --git a/c_src/Makefile b/c_src/Makefile
index aa380cb..72a47d8 100644
--- a/c_src/Makefile
+++ b/c_src/Makefile
@@ -21,4 +21,4 @@ hsmhelper: $(hsmhelper_OBJS)
 	$(CC) -o hsmhelper $(hsmhelper_OBJS) -ldl $(LDFLAGS)
 
 permdbport: $(permdbport_OBJS)
-	$(CC) -o permdbport $(permdbport_OBJS) -ldl $(LDFLAGS)
+	$(CC) -o permdbport $(permdbport_OBJS) $(LDFLAGS)
diff --git a/c_src/permdb.c b/c_src/permdb.c
index 4935c77..7a58885 100644
--- a/c_src/permdb.c
+++ b/c_src/permdb.c
@@ -18,19 +18,18 @@
 #include "permdb.h"
 #include "hash.h"
 
-const int bytesperentry = 8;
-const int bitsperlevel = 2;
-const int keylen = 32;
+static const int bitsperlevel = 2;
+static const int keylen = 32;
 
-const char *nodemagic = "\x8a\x44";
-const char *datamagic = "\xcb\x0e";
+static const char *nodemagic = "\x8a\x44";
+static const char *datamagic = "\xcb\x0e";
 
 typedef struct {
         int fd;
         node_offset datasize;
         node_offset lastcommit;
         node_offset filesize;
-        void *writebuffer;
+        char *writebuffer;
         uint64_t writebufferalloc;
 } buffered_file;
 
@@ -42,12 +41,13 @@ struct permdb_object {
         char *error;
 };
 
-node_object nullnode = {{0, 0, 0, 0}};
+static const node_object nullnode = {{0, 0, 0, 0}};
 
 
-char indexfile_header[16] = "PERMDB IDX FILE ";
+static const char indexfile_header[16] = "PERMDB IDX FILE ";
 
-void
+#if 0
+static void
 print_entry(node_object node)
 {
         for (int i = 0; i < entriespernode; i++) {
@@ -55,9 +55,10 @@ print_entry(node_object node)
         }
         fprintf(stderr, "\n");
 }
+#endif
 
 static void
-writebuffer_add(buffered_file *file, const void *data, long length);
+writebuffer_add(buffered_file *file, const void *data, uint64_t length);
 static int
 writebuffer_flush(buffered_file *file);
 
@@ -66,7 +67,7 @@ struct nodecache {
         char key[];
 };
 
-int
+static int
 comparenodes(void *a_v, void *b_v)
 {
         struct nodecache *a = (struct nodecache *)a_v;
@@ -77,7 +78,7 @@ comparenodes(void *a_v, void *b_v)
         return strcmp(a->key, b->key);
 }
 
-unsigned
+static unsigned
 hashnode(void *node_v)
 {
         struct nodecache *node = (struct nodecache *)node_v;
@@ -111,7 +112,7 @@ print_hex(const void *data, int length)
 #define DEBUG_WRITE 0
 #define DEBUG_READ 0
 
-node_object
+static node_object
 get_node_from_cache(permdb_object *state, const char *key)
 {
 #if DEBUG_CACHE
@@ -136,7 +137,7 @@ get_node_from_cache(permdb_object *state, const char *key)
         return node->value;
 }
 
-node_object
+static node_object
 get_node_from_dirtynodes(permdb_object *state, const char *key)
 {
 #if DEBUG_CACHE
@@ -161,7 +162,7 @@ get_node_from_dirtynodes(permdb_object *state, const char *key)
         return node->value;
 }
 
-void
+static void
 put_node_in_cache(permdb_object *state, const char *key, node_object value)
 {
 #if DEBUG_CACHE
@@ -174,7 +175,7 @@ put_node_in_cache(permdb_object *state, const char *key, node_object value)
         hashtabaddreplace(state->nodecache, node);
 }
 
-void
+static void
 put_node_in_dirtynodes(permdb_object *state, const char *key, node_object value)
 {
 #if DEBUG_CACHE
@@ -193,7 +194,7 @@ true_cond(void *ptr, void *arg)
     return TRUE;
 }
 
-int
+static int
 free_hash_node(void *ptr, void *arg)
 {
         free(ptr);
@@ -246,12 +247,22 @@ permdb_alloc(const char *dbpath)
         state->indexfile.fd = idxfd;
         state->nodecache = hashtabnewf(1000000, comparenodes, hashnode, HASHTAB_GROW);
         state->dirtynodes = hashtabnewf(1000000, comparenodes, hashnode, HASHTAB_GROW);
-        state->datafile.filesize = lseek(fd, 0, SEEK_END);
+        off_t datafile_filesize = lseek(fd, 0, SEEK_END);
+        if (datafile_filesize < 0) {
+                warn("lseek %s", dbpath);
+                return NULL;
+        }
+        state->datafile.filesize = (node_offset) datafile_filesize;
         state->datafile.datasize = state->datafile.filesize;
         state->datafile.lastcommit = state->datafile.datasize;
         state->datafile.writebufferalloc = 1024*1024;
         state->datafile.writebuffer = calloc(state->datafile.writebufferalloc, 1);
-        state->indexfile.filesize = lseek(idxfd, 0, SEEK_END);
+        off_t indexfile_filesize = lseek(idxfd, 0, SEEK_END);
+        if (indexfile_filesize < 0) {
+                warn("lseek %s", idxpath);
+                return NULL;
+        }
+        state->indexfile.filesize = (node_offset) indexfile_filesize;
         state->indexfile.datasize = state->indexfile.filesize;
         state->indexfile.lastcommit = state->indexfile.datasize;
         state->indexfile.writebufferalloc = 1024*1024;
@@ -288,19 +299,19 @@ permdb_free(permdb_object *state)
         free(state);
 }
 
-long
+static uint64_t
 writebuffer_length(buffered_file *file)
 {
-        return file->datasize - file->filesize;
+        return (uint64_t) file->datasize - (uint64_t) file->filesize;
 }
 
 static int
 writebuffer_flush_nosync(buffered_file *file);
 
 static void
-writebuffer_add(buffered_file *file, const void *data, long length)
+writebuffer_add(buffered_file *file, const void *data, uint64_t length)
 {
-        long needspace = length + writebuffer_length(file);
+        uint64_t needspace = length + writebuffer_length(file);
 
         if (needspace > file->writebufferalloc) {
                 writebuffer_flush_nosync(file);
@@ -309,7 +320,7 @@ writebuffer_add(buffered_file *file, const void *data, long length)
 
                 if (needspace > file->writebufferalloc) {
 
-                        long newsize = file->writebufferalloc * 2;
+                        uint64_t newsize = file->writebufferalloc * 2;
                         if (needspace > newsize) {
                                 newsize = needspace;
                         }
@@ -326,14 +337,14 @@ writebuffer_add(buffered_file *file, const void *data, long length)
 static int
 writebuffer_flush_nosync(buffered_file *file)
 {
-        int ret;
+        ssize_t ret;
 
-        int length = writebuffer_length(file);
+        uint64_t length = writebuffer_length(file);
         ret = write(file->fd, file->writebuffer, length);
         if (ret != length) {
                 return -1;
         }
-        file->filesize += ret;
+        file->filesize += (node_offset) ret;
         return 0;
 }
 
@@ -352,30 +363,30 @@ writebuffer_flush(buffered_file *file)
 }
 
 static unsigned char
-keybits(const char *key, unsigned int level)
+keybits(const unsigned char *key, unsigned int level)
 {
     unsigned char b = key[level/4];
     
     return (b >> (6-(level*2) % 8)) & 0x3;
 }
 
-char *
-keypart(const char *key, unsigned int level)
+static char *
+keypart(const unsigned char *key, unsigned int level)
 {
-    char *result = malloc(level+1);
+    unsigned char *result = malloc(level+1);
 
-    int i;
+    unsigned int i;
     for (i = 0; i < level; i++) {
             unsigned char b = keybits(key, i);
-            result[i] = b + '0';
+            result[i] = b + (unsigned char) '0';
     }
 
     result[level] = 0;
 
-    return result;
+    return (char *)result;
 }
 
-char *
+static char *
 packnode(node_object node)
 {
         char *data = malloc(strlen(nodemagic) + sizeof(node_object));
@@ -385,16 +396,18 @@ packnode(node_object node)
         return data;
 }
 
-void
-addentry(node_object *node, unsigned char n, node_entry entry)
+static void
+addentry(node_object *node, unsigned int n, node_entry entry)
 {
+        assert(n < entriespernode);
         assert(node->data[n] == 0);
         node->data[n] = entry;
 }
 
-void
-overwriteentry(node_object *node, unsigned char n, node_entry entry)
+static void
+overwriteentry(node_object *node, unsigned int n, node_entry entry)
 {
+        assert(n < entriespernode);
         assert(node->data[n] != 0);
         node->data[n] = entry;
 }
@@ -430,7 +443,7 @@ set_error(permdb_object *state, const char *format, ...)
 }
 
 static node_object
-unpacknode(permdb_object *state, const char *data, int datalen)
+unpacknode(permdb_object *state, const char *data, size_t datalen)
 {
         if (memcmp(nodemagic, data, 2) != 0) {
                 set_error(state, "incorrect magic %02x%02x\n", (unsigned char)data[0], (unsigned char)data[1]);
@@ -441,15 +454,17 @@ unpacknode(permdb_object *state, const char *data, int datalen)
                 return nullnode;
         }
 
-        node_object *node = (node_object *)(data + 2);
+        node_object node;
 
-        return *node;
+        memcpy(&node, data + 2, sizeof(node));
+
+        return node;
 }
 
-char *
-read_internal_data(permdb_object *state, node_offset offset, unsigned int length)
+unsigned char *
+read_internal_data(permdb_object *state, node_offset offset, size_t length)
 {
-    char *result = malloc(length);
+    unsigned char *result = malloc(length);
     buffered_file *file = &state->datafile;
 #if DEBUG_READ
     fprintf(stderr, "reading data: offset %llu\n", offset);
@@ -458,20 +473,20 @@ read_internal_data(permdb_object *state, node_offset offset, unsigned int length
     if (offset >= file->filesize) {
             node_offset writebufferoffset = offset - file->filesize;
             if (offset + length > file->datasize) {
-                    set_error(state, "pread: not enough data for offset %llu and length %u\n", (long long unsigned int) offset, length);
+                    set_error(state, "pread: not enough data for offset %llu and length %zu\n", (long long unsigned int) offset, length);
                     return NULL;
             }
             memcpy(result, file->writebuffer + writebufferoffset, length);
     } else {
             if (offset + length > file->filesize) {
-                    set_error(state, "pread: trying to read over file/writebuffer boundary for offset %llu and length %u\n", (long long unsigned int) offset, length);
+                    set_error(state, "pread: trying to read over file/writebuffer boundary for offset %llu and length %zu\n", (long long unsigned int) offset, length);
                     return NULL;
             }
             
-            int ret = pread(file->fd, result, length, offset);
+            ssize_t ret = pread(file->fd, result, length, (off_t) offset);
             if (ret != length) {
                     free(result);
-                    set_error(state, "short pread: %u (wanted %u) at offset %llu\n", ret, length, (long long unsigned int) offset);
+                    set_error(state, "short pread: %zd (wanted %zu) at offset %llu\n", ret, length, (long long unsigned int) offset);
                     return NULL;
             }
     }
@@ -479,7 +494,7 @@ read_internal_data(permdb_object *state, node_offset offset, unsigned int length
     return result;
 }
 
-int
+static int
 isnullnode(node_object node)
 {
         return node.data[0] == 0 && node.data[1] == 0 && node.data[2] == 0 && node.data[3] == 0;
@@ -512,13 +527,16 @@ readnode(permdb_object *state, node_offset offset, const char *cachekey)
                 }
         }
 
-        int length = strlen(nodemagic) + sizeof(node_object);
+        size_t length = strlen(nodemagic) + sizeof(node_object);
 
         char *buffer = malloc(length);
-        int ret = pread(state->indexfile.fd, buffer, length, offset);
+        if (buffer == NULL) {
+                return nullnode;
+        }
+        ssize_t ret = pread(state->indexfile.fd, buffer, length, (off_t) offset);
         if (ret != length) {
                 free(buffer);
-                set_error(state, "node not present at %llu: length %d\n", (long long unsigned int) offset, ret);
+                set_error(state, "node not present at %llu: length %zd\n", (long long unsigned int) offset, ret);
                 return nullnode;
         }
         
@@ -534,13 +552,13 @@ readnode(permdb_object *state, node_offset offset, const char *cachekey)
 }
 
 
-int
+static int
 isdata(node_entry entry)
 {
         return (entry & NODE_ENTRY_ISDATA) == NODE_ENTRY_ISDATA;
 }
 
-node_offset
+static node_offset
 entryoffset(node_entry entry)
 {
         return entry & NODE_ENTRY_OFFSET_MASK;
@@ -548,18 +566,18 @@ entryoffset(node_entry entry)
 
 struct nodelist {
         node_object *nodes;
-        int len;
-        int pos;
+        unsigned int len;
+        unsigned int pos;
 };
 
-void
+static void
 add_entry_to_nodelist(struct nodelist *list, node_object node)
 {
         list->nodes[list->pos] = node;
         list->pos++;
 }
 
-void
+static void
 init_nodelist(struct nodelist *list)
 {
         list->len = keylen * 8 / bitsperlevel;
@@ -567,14 +585,14 @@ init_nodelist(struct nodelist *list)
         list->pos = 0;
 }
 
-void
+static void
 free_nodelist(struct nodelist *list)
 {
         free(list->nodes);
 }
 
 static char
-getpath(permdb_object *state, const char *key, struct nodelist *nodes)
+getpath(permdb_object *state, const unsigned char *key, struct nodelist *nodes)
 {
         unsigned int level = 0;
 
@@ -601,7 +619,7 @@ getpath(permdb_object *state, const char *key, struct nodelist *nodes)
                 }
                 add_entry_to_nodelist(nodes, node);
                 if (entry == 0 || isdata(entry)) {
-                        return kb;
+                        return (char) kb;
                 }
                 level++;
                 char *kp = keypart(key, level);
@@ -616,7 +634,7 @@ getpath(permdb_object *state, const char *key, struct nodelist *nodes)
 }
 
 static node_entry
-getpathlastnode(permdb_object *state, const char *key)
+getpathlastnode(permdb_object *state, const unsigned char *key)
 {
         unsigned int level = 0;
 
@@ -644,7 +662,7 @@ getpathlastnode(permdb_object *state, const char *key)
         return entry;
 }
 
-static long
+static node_offset
 writenode(permdb_object *state, node_object node, const char *cachekey)
 {
         node_offset offset = state->indexfile.datasize;
@@ -670,7 +688,7 @@ datasize(permdb_object *state)
 }
 
 
-node_entry
+static node_entry
 buildentry(int isdata, node_offset offset)
 {
         if (isdata) {
@@ -681,17 +699,17 @@ buildentry(int isdata, node_offset offset)
 }
 
 static void *
-memsub(void *src, int offset, int length)
+memsub(void *src, size_t offset, size_t length)
 {
         void *result = malloc(length);
-        memcpy(result, src + offset, length);
+        memcpy(result, ((char *)src) + offset, length);
         return result;
 }
 
-static char *
+static unsigned char *
 readdatakey(permdb_object *state, node_offset offset)
 {
-        char *data = read_internal_data(state, offset, strlen(datamagic) + keylen);
+        unsigned char *data = read_internal_data(state, offset, strlen(datamagic) + keylen);
         if (data == NULL) {
                 return NULL;
         }
@@ -700,15 +718,25 @@ readdatakey(permdb_object *state, node_offset offset)
                 set_error(state, "incorrect magic %02x %02x\n", (unsigned char)data[0], (unsigned char)data[1]);
                 return NULL;
         }
-        char *result = memsub(data, strlen(datamagic), keylen);
+        unsigned char *result = memsub(data, strlen(datamagic), keylen);
         free(data);
         return result;
 }
 
-static char *
-readdatakeyandlen(permdb_object *state, node_offset offset, unsigned int *datalen)
+static uint32_t
+readnet32(void *ptr)
 {
-        char *data = read_internal_data(state, offset, strlen(datamagic) + keylen + 4);
+        uint32_t data;
+
+        memcpy(&data, ptr, sizeof(data));
+
+        return ntohl(data);
+}
+
+static unsigned char *
+readdatakeyandlen(permdb_object *state, node_offset offset, size_t *datalen)
+{
+        unsigned char *data = read_internal_data(state, offset, strlen(datamagic) + keylen + 4);
         if (data == NULL) {
                 return NULL;
         }
@@ -717,21 +745,21 @@ readdatakeyandlen(permdb_object *state, node_offset offset, unsigned int *datale
                 set_error(state, "incorrect magic %02x %02x\n", (unsigned char)data[0], (unsigned char)data[1]);
                 return NULL;
         }
-        char *result = memsub(data, strlen(datamagic), keylen);
-        *datalen = ntohl(*(uint32_t *)(data+strlen(datamagic)+keylen));
+        unsigned char *result = memsub(data, strlen(datamagic), keylen);
+        *datalen = readnet32(data+strlen(datamagic)+keylen);
         free(data);
         return result;
 }
 
-static char *
-readdata(permdb_object *state, node_offset offset, unsigned int datalen)
+static unsigned char *
+readdata(permdb_object *state, node_offset offset, size_t datalen)
 {
         return read_internal_data(state, offset + strlen(datamagic) + keylen + 4, datalen);
 }
 
 
 static node_offset
-writedata(permdb_object *state, const char *key, const char *data, unsigned int datalength)
+writedata(permdb_object *state, const unsigned char *key, const unsigned char *data, size_t datalength)
 {
         uint32_t coded_datalength = htonl(datalength);
         node_offset offset = state->datafile.datasize;
@@ -748,7 +776,7 @@ writedata(permdb_object *state, const char *key, const char *data, unsigned int
 }
 
 int
-addvalue(permdb_object *state, const char *key, unsigned int keylength, const char *data, unsigned int datalength)
+addvalue(permdb_object *state, const unsigned char *key, unsigned int keylength, const unsigned char *data, size_t datalength)
 {
         struct nodelist nodes;
         init_nodelist(&nodes);
@@ -760,7 +788,7 @@ addvalue(permdb_object *state, const char *key, unsigned int keylength, const ch
                 return -1;
         }
 
-        int foundlevel = nodes.pos - 1;
+        unsigned int foundlevel = nodes.pos - 1;
 
         if (foundlevel >= nodes.len) {
                 fprintf(stderr, "tried to read after end of allocated list\n");
@@ -768,12 +796,12 @@ addvalue(permdb_object *state, const char *key, unsigned int keylength, const ch
                 return 0;
         }
         node_object lastnode = nodes.nodes[foundlevel];
-        if (get_entry_in_node(lastnode, kb) == 0) {
+        if (get_entry_in_node(lastnode, (unsigned char) kb) == 0) {
                 node_offset dataoffset = writedata(state, key, data, datalength);
                 addentry(&lastnode, keybits(key, foundlevel), buildentry(1, dataoffset));
         } else {
-                node_offset olddataoffset = entryoffset(get_entry_in_node(lastnode, kb));
-                char *olddatakey = readdatakey(state, olddataoffset);
+                node_offset olddataoffset = entryoffset(get_entry_in_node(lastnode, (unsigned char) kb));
+                unsigned char *olddatakey = readdatakey(state, olddataoffset);
                 if (olddatakey == NULL) {
                         free_nodelist(&nodes);
                         return -1;
@@ -784,7 +812,7 @@ addvalue(permdb_object *state, const char *key, unsigned int keylength, const ch
                         return 0;
                 }
                 node_offset dataoffset = writedata(state, key, data, datalength);
-                int level = foundlevel + 1;
+                unsigned int level = foundlevel + 1;
                 while (keybits(key, level) == keybits(olddatakey, level)) {
                         level++;
                 }
@@ -792,9 +820,11 @@ addvalue(permdb_object *state, const char *key, unsigned int keylength, const ch
                 addentry(&leafnode, keybits(key, level), buildentry(1, dataoffset));
                 addentry(&leafnode, keybits(olddatakey, level), buildentry(1, olddataoffset));
                 free(olddatakey);
-                char *cachekey = keypart(key, level);
-                put_node_in_dirtynodes(state, cachekey, leafnode);
-                free(cachekey);
+                {
+                        char *cachekey = keypart(key, level);
+                        put_node_in_dirtynodes(state, cachekey, leafnode);
+                        free(cachekey);
+                }
                 level--;
                 while (level > foundlevel) {
                         node_object node = nullnode;
@@ -807,22 +837,24 @@ addvalue(permdb_object *state, const char *key, unsigned int keylength, const ch
                 overwriteentry(&lastnode, keybits(key, foundlevel), NODE_ENTRY_DIRTY_NODE);
         }
 
-        int level = foundlevel;
+        int level = (int) foundlevel;
 
-        char *cachekey = keypart(key, level);
-        put_node_in_dirtynodes(state, cachekey, lastnode);
-        free(cachekey);
+        {
+                char *cachekey = keypart(key, (unsigned int) level);
+                put_node_in_dirtynodes(state, cachekey, lastnode);
+                free(cachekey);
+        }
 
         level--;
         while (level >= 0) {
-                if (level >= nodes.len) {
+                if (level >= (int) nodes.len) {
                         fprintf(stderr, "tried to read after end of allocated list\n");
                         free_nodelist(&nodes);
                         return 0;
                 }
                 node_object node = nodes.nodes[level];
-                overwriteentry(&node, keybits(key, level), NODE_ENTRY_DIRTY_NODE);
-                char *cachekey = keypart(key, level);
+                overwriteentry(&node, keybits(key, (unsigned int) level), NODE_ENTRY_DIRTY_NODE);
+                char *cachekey = keypart(key, (unsigned int) level);
                 put_node_in_dirtynodes(state, cachekey, node);
                 free(cachekey);
                 level--;
@@ -833,8 +865,8 @@ addvalue(permdb_object *state, const char *key, unsigned int keylength, const ch
         return 1;
 }
 
-char *
-getvalue(permdb_object *state, const char *key, int keylen, unsigned int *datalen)
+unsigned char *
+getvalue(permdb_object *state, const unsigned char *key, size_t keylength, size_t *datalen)
 {
         node_entry entry = getpathlastnode(state, key);
         if (entry == 0) {
@@ -843,11 +875,11 @@ getvalue(permdb_object *state, const char *key, int keylen, unsigned int *datale
 
         node_offset olddataoffset = entryoffset(entry);
 
-        char *datakey = readdatakeyandlen(state, olddataoffset, datalen);
+        unsigned char *datakey = readdatakeyandlen(state, olddataoffset, datalen);
         if (datakey == NULL) {
                 return NULL;
         }
-        if (memcmp(datakey, key, keylen) != 0) {
+        if (memcmp(datakey, key, keylength) != 0) {
                 free(datakey);
                 return NULL;
         }
@@ -860,7 +892,7 @@ struct keylist {
         int pos;
 };
 
-int
+static int
 add_entry_to_keylist(void *ptr, void *arg)
 {
         struct keylist *list = (struct keylist *) arg;
@@ -871,15 +903,23 @@ add_entry_to_keylist(void *ptr, void *arg)
         return 0;
 }
 
-int 
+static int 
 string_length_comparison(const void *a_v, const void *b_v) {
-        char **a = (char **) a_v;
-        char **b = (char **) b_v;
-        return strlen(*b) - strlen(*a);
+        char * const *a = (char * const *) a_v;
+        char * const *b = (char * const *) b_v;
+        size_t a_len = strlen(*a);
+        size_t b_len = strlen(*b);
+        if (b_len > a_len) {
+                return 1;
+        } else if (b_len == a_len) {
+                return 0;
+        } else {
+                return -1;
+        }
 }
 
-char **
-sorted(permdb_object *state, int ndirtynodes)
+static char **
+sorted(permdb_object *state, unsigned int ndirtynodes)
 {
         struct keylist keylist;
         keylist.keys = malloc(sizeof(char *) * ndirtynodes);
@@ -903,7 +943,7 @@ sorted(permdb_object *state, int ndirtynodes)
         return keylist.keys;
 }
 
-int
+static int
 count_entry(void *ptr, void *arg)
 {
         int *flag = (int *)arg;
@@ -915,7 +955,7 @@ int
 committree(permdb_object *state)
 {
         get_node_from_dirtynodes(state, "");
-        int ndirtynodes = 0;
+        unsigned int ndirtynodes = 0;
         hashtabforeach(state->dirtynodes, count_entry, &ndirtynodes);
         if (ndirtynodes == 0) {
                 return 0;
@@ -930,8 +970,8 @@ committree(permdb_object *state)
                 return -1;
         }
         
-        int ncommits = ndirtynodes;
-        int i;
+        unsigned int ncommits = ndirtynodes;
+        unsigned int i;
         for (i = 0; i < ncommits; i++) {
                 get_node_from_dirtynodes(state, "");
                 char *key = commitlist[i];
@@ -941,12 +981,12 @@ committree(permdb_object *state)
                 assert(get_entry_in_node(node, 2) != NODE_ENTRY_DIRTY_NODE);
                 assert(get_entry_in_node(node, 3) != NODE_ENTRY_DIRTY_NODE);
                 node_offset offset = writenode(state, node, key);
-                int keylength = strlen(key);
+                size_t keylength = strlen(key);
                 if (keylength != 0) {
                         get_node_from_dirtynodes(state, "");
                         char *parent = strdup(key);
                         parent[keylength - 1] = '\0';
-                        int entrynumber = key[keylength - 1] - '0';
+                        unsigned int entrynumber = (unsigned int) (key[keylength - 1] - '0');
                         node_object parentnode = get_node_from_dirtynodes(state, parent);
                         overwriteentry(&parentnode, entrynumber, offset);
                         put_node_in_dirtynodes(state, parent, parentnode);
@@ -981,9 +1021,9 @@ committree(permdb_object *state)
 void
 portloop(permdb_object *state)
 {
-        char buf[65536];
+        unsigned char buf[65536];
         ssize_t len;
-        while ((len = read_command((unsigned char *)buf, sizeof(buf)-1, 4)) > 0) {
+        while ((len = read_command(buf, sizeof(buf)-1, 4)) > 0) {
                 if (buf[0] == 0) {
                         if (len != keylen+1) {
                                 write_reply(NULL, 0, 4);
@@ -991,32 +1031,38 @@ portloop(permdb_object *state)
                         }
                         assert(len == keylen+1);
 
-                        unsigned int datalen;
-                        char *result = getvalue(state, (buf+1), keylen, &datalen);
+                        size_t datalen;
+                        unsigned char *result = getvalue(state, (buf+1), keylen, &datalen);
 
                         if (result == NULL) {
                                 write_reply(NULL, 0, 4);
                         } else {
-                                write_reply((unsigned char *)result, datalen, 4);
+                                write_reply(result, datalen, 4);
                         }
                         free(result);
                 } else if (buf[0] == 1) {
-                        unsigned int datalen = len - keylen - 1;
-                        char *key = buf + 1;
-                        char *data = key + keylen;
+                        if (len < (keylen + 1)) {
+                                write_reply(NULL, 0, 4);
+                                fprintf(stderr, "invalid addvalue command, length was %zd\n", len);
+                                continue;
+                        }
+
+                        size_t datalen = (size_t) (len - keylen - 1);
+                        unsigned char *key = buf + 1;
+                        unsigned char *data = key + keylen;
 
                         int result = addvalue(state, key, keylen, data, datalen);
 
                         if (result < 0) {
                                 write_reply(NULL, 0, 4);
                         } else {
-                                unsigned char result_byte = result;
+                                unsigned char result_byte = (unsigned char) result;
                                 write_reply(&result_byte, 1, 4);
                         }
                 } else if (buf[0] == 2) {
                         if (len != 1) {
                                 write_reply(NULL, 0, 4);
-                                fprintf(stderr, "invalid commit command, length was %ld\n", len);
+                                fprintf(stderr, "invalid commit command, length was %zd\n", len);
                                 continue;
                         }
 
@@ -1026,7 +1072,7 @@ portloop(permdb_object *state)
                                 write_reply(NULL, 0, 4);
                                 continue;
                         } else {
-                                unsigned char result_byte = result;
+                                unsigned char result_byte = (unsigned char) result;
                                 write_reply(&result_byte, 1, 4);
                         }                        
                 } else {
diff --git a/c_src/permdb.h b/c_src/permdb.h
index ee1cd66..ded6754 100644
--- a/c_src/permdb.h
+++ b/c_src/permdb.h
@@ -26,8 +26,8 @@ typedef struct permdb_object permdb_object;
 node_entry
 get_entry_in_node(node_object node, unsigned char n);
 
-char *
-read_internal_data(permdb_object *state, node_offset offset, unsigned int length);
+unsigned char *
+read_internal_data(permdb_object *state, node_offset offset, size_t length);
 
 node_object
 readnode(permdb_object *state, node_offset offset, const char *cachekey);
@@ -36,10 +36,10 @@ node_offset
 datasize(permdb_object *state);
 
 int
-addvalue(permdb_object *state, const char *key, unsigned int keylength, const char *data, unsigned int datalength);
+addvalue(permdb_object *state, const unsigned char *key, unsigned int keylength, const unsigned char *data, size_t datalength);
 
-char *
-getvalue(permdb_object *state, const char *key, int keylen, unsigned int *datalen);
+unsigned char *
+getvalue(permdb_object *state, const unsigned char *key, size_t keylength, size_t *datalen);
 
 void
 delete_all_nodes_in_cache(permdb_object *state);
diff --git a/c_src/permdbport.c b/c_src/permdbport.c
index 4215d68..8f7dd7c 100644
--- a/c_src/permdbport.c
+++ b/c_src/permdbport.c
@@ -14,7 +14,7 @@
 #include "permdb.h"
 #include "erlport.h"
 
-static void
+static void __attribute__((noreturn))
 usage()
 {
         errx(1, "usage: permdbport <path>");
@@ -32,6 +32,7 @@ main(int argc, char *argv[])
 
         if (state == NULL) {
                 write_reply(NULL, 0, 4);
+                return 1;
         }
 
         portloop(state);