add unit tests

mduptab.h

@@ -1,3 +1,6 @@
+#ifndef _MDUPTAB_H
+#define _MDUPTAB_H
+
 /* save memory for constant strings by keeping a list of the ones that
  * we already saw and not allocating memory for each new one.  The only
  * API is "add string and return offset".  The offset is relative to the
@@ -5,6 +8,10 @@
  * If the same string was already there, it will return offset of that
  * string, otherwise it will insert a copy of the new string. */
 
+/* This is not performance optimized. It is used for objectclass entries
+ * in tinyldap. You will typically have less than a dozen different
+ * objectclasses. */
+
 #include "mstorage.h"
 
 typedef struct mduptable {
@@ -14,6 +21,8 @@ typedef struct mduptable {
 
 void mduptab_init(mduptab_t* t);
 void mduptab_init_reuse(mduptab_t* t,mstorage_t* s);
-long mduptab_add(mduptab_t* t,const char* s,size_t len);
-long mduptab_adds(mduptab_t* t,const char* s);
+ssize_t mduptab_add(mduptab_t* t,const char* s,size_t len);
+ssize_t mduptab_adds(mduptab_t* t,const char* s);
 void mduptab_reset(mduptab_t* t);
+
+#endif

mduptab_add.c

@@ -1,3 +1,8 @@
+#ifdef UNITTEST
+// if we want to test the mremap-version of mstorage under Linux, we
+// need to have _GNU_SOURCE defined otherwise MREMAP_MAYMOVE is missing
+#define _GNU_SOURCE
+#endif
 #include <stdlib.h>
 #include <string.h>
 #include <libowfat/str.h>
@@ -6,18 +11,55 @@
 #include "mduptab.h"
 #include <libowfat/uint32.h>
 
-long mduptab_add(mduptab_t* t,const char* s,size_t len) {
+#ifdef UNITTEST
+static int failafter;
+ssize_t fail_mstorage_add(mstorage_t* p,const char* s,size_t n) {
+  if (--failafter == 0)
+    return -1;
+  return mstorage_add(p,s,n);
+}
+#define mstorage_add fail_mstorage_add
+#endif
+
+ssize_t mduptab_add(mduptab_t* t,const char* s,size_t len) {
   unsigned int i;
   unsigned long* l=(unsigned long*)t->table.root;
-  long x,bak;
+  ssize_t x,bak;
   for (i=0; i<t->table.used/sizeof(unsigned long); ++i)
     if (bstr_equal2(t->Strings->root+l[i],s,len))
       return l[i];
   bak=t->Strings->used;
-  if ((x=mstorage_add_bin(t->Strings,s,len))<0) return -1;
+  if ((x=mstorage_add_bin(t->Strings,s,len))<0)
+    return -1;
   if (mstorage_add(&t->table,(const char*)&x,sizeof(x))<0) {
     t->Strings->used=bak;
     return -1;
   }
   return x;
 }
+
+#ifdef UNITTEST
+#undef mstorage_add
+#include <assert.h>
+#undef UNITTEST
+#include "mduptab_init.c"
+#include "mstorage_init.c"
+#include "mstorage_add.c"
+#include "mstorage_add_bin.c"
+#include "mduptab_reset.c"
+#include "mstorage_unmap.c"
+#include "bstr_diff2.c"
+
+int main() {
+  mduptab_t t;
+  mduptab_init(&t);
+  assert(mduptab_add(&t,"foo",3) == 0);
+  assert(mduptab_add(&t,"bar",3) == 4);
+  assert(mduptab_add(&t,"foo",3) == 0);
+  assert(mduptab_add(&t,"bar",3) == 4);
+  assert(mduptab_add(&t, "foo", (size_t)-1) == -1);
+  failafter=1;
+  assert(mduptab_add(&t, "baz", 3) == -1);
+  mduptab_reset(&t);
+}
+#endif

mduptab_init_reuse.c

@@ -1,3 +1,5 @@
+/* This is no longer used by tinyldap, but there is a reference in
+ * old-parse.c so I'm keeping the source file in the repository */
 #include "mduptab.h"
 
 void mduptab_init_reuse(mduptab_t* t,mstorage_t* s) {

mstorage.h

@@ -2,6 +2,7 @@
 #define _MSTORAGE_H
 
 #include <stddef.h>
+#include <sys/types.h>
 
 /* (optionally persistent) mmapped storage. */
 
@@ -18,16 +19,20 @@ int mstorage_init_persistent(mstorage_t* p,int fd);
 /* Works like strstorage_add, but will return an
  * offset to mstorage_root, which is mmapped and may thus change. */
 /* offset -1 ==> error */
-long mstorage_add(mstorage_t* p,const char* s,size_t n);
+ssize_t mstorage_add(mstorage_t* p,const char* s,size_t n);
+/* same as mstorage_add but does not do the byte_copy */
+ssize_t mstorage_reserve(mstorage_t* p,size_t n);
 
-/* undo mapping */
-void mstorage_unmap(mstorage_t* p);
+/* undo mapping. return 0 on success, -1 on failure */
+int mstorage_unmap(mstorage_t* p);
 
 /* this is tinyldap specific.  If the data contains at least one 0-byte,
  * it is stored in a tinyldap specific encoding:
  *   char 0;
  *   uint32 len;
  *   char data[len] */
-long mstorage_add_bin(mstorage_t* p,const char* s,size_t n);
+// the name is slightly confusing. It stores a bstr (bstr.h)
+// return offset (>=0) relative to p->root on success, -1 on fail
+ssize_t mstorage_add_bin(mstorage_t* p,const char* s,size_t n);
 
 #endif

mstorage_add.c

@@ -5,7 +5,7 @@
 #include <fcntl.h>
 #include <sys/shm.h>
 #include <stdio.h>
-#include <libowfat/byte.h>
+#include <string.h>
 #include "mstorage.h"
 
 #ifndef PAGE_SIZE
@@ -19,16 +19,21 @@ unsigned long mstorage_increment=4*PAGE_SIZE;
 /* Sadly, mremap is only available on Linux */
 /* Please petition your congressman^Woperating system vendor to include it! */
 
-long mstorage_add(mstorage_t* p,const char* s,size_t n) {
+ssize_t mstorage_reserve(mstorage_t* p,size_t n) {
   if (p->mapped-p->used<n) {
+    size_t need;
     if (!p->root) {
       /* nothing allocated.  mmap /dev/zero */
       char* tmp;
-      long need=(n|PAGEMASK)+1;
+      need=(n|PAGEMASK)+1;
+      if (need==0)	// integer overflow
+	return -1;	// #intof1
+      if ((ssize_t)need < 0)
+	return -1;	// #inttr1
 #ifdef MREMAP_MAYMOVE
 #ifdef MAP_ANONYMOUS
       if ((tmp=mmap(0,need,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0))==MAP_FAILED)
-	return -1;
+	return -1;	// #mmap1
 #else
       int fd=open("/dev/zero",O_RDWR);
       if (fd<0) return -1;
@@ -45,41 +50,105 @@ long mstorage_add(mstorage_t* p,const char* s,size_t n) {
       p->mapped=need;
       p->used=0;
     } else {
-      long need=((p->used+n)|PAGEMASK)+1+mstorage_increment;
+      need = p->used+n;
+      if (need < n)	// integer overflow
+	return -1;	// #intof2
+      need=(need|PAGEMASK)+1+mstorage_increment;
+      if (need <= mstorage_increment)	// integer overflow
+	return -1;	// #intof3
+      if ((ssize_t)need < 0)
+	return -1;	// #inttr2
       char* tmp;
 #ifdef MREMAP_MAYMOVE
       tmp=mremap(p->root,p->mapped,need,MREMAP_MAYMOVE);
-      if (tmp==MAP_FAILED) return -1;
+      if (tmp==MAP_FAILED)
+	return -1;	// #mmap2
 #else
       if (p->fd==-1) {
 	tmp=realloc(p->root,need);
-	if (!tmp) return -1;
+	if (!tmp)
+	  return -1;
       } else {
 	munmap(p->root,p->used);
+	p->root=0;
 	tmp=mmap(0,need,PROT_READ|PROT_WRITE,MAP_SHARED,p->fd,0);
-	if (tmp==-1) {
+	if (tmp==MAP_FAILED) {
 	  tmp=mmap(0,p->used,PROT_READ|PROT_WRITE,MAP_SHARED,p->fd,0);
-	  /* this can never fail, because we mmap exactly as much as we
+	  /* this should never fail, because we mmap exactly as much as we
 	   * had mmapped previously.  We need to munmap before doing the
 	   * new mmap, though, because we may run into the address space
 	   * limit too early on 32-bit systems with lots of RAM */
+	  if (tmp!=MAP_FAILED)
+	    p->root=tmp;	// fail but leave p in usable state
 	  return -1;
 	}
       }
 #endif
-      if (p->fd!=-1) {
-	/* slight complication if the storage is file based: we need to
-	  * make sure the file size is extended, or the byte_copy will
-	  * yield a bus error. */
-	if (ftruncate(p->fd,need)==-1) return -1;
-      }
-      p->mapped=need; p->root=tmp;
+      p->root=tmp;
     }
+    if (p->fd!=-1) {
+      /* slight complication if the storage is file based: we need to
+	* make sure the file size is extended, or the byte_copy will
+	* yield a bus error. */
+      if (ftruncate(p->fd,need)==-1)
+	return -1;	// #trunc
+    }
+    p->mapped=need;
   }
-  byte_copy(p->root+p->used,n,s);
-  {
-    unsigned long l=p->used;
-    p->used+=n;
-    return l;
-  }
+  ssize_t l=p->used;
+  p->used+=n;
+  return l;
 }
+
+ssize_t mstorage_add(mstorage_t* p,const char* s,size_t n) {
+  ssize_t l = mstorage_reserve(p,n);
+  if (l>=0)
+    memcpy(p->root+l,s,n);
+  return l;
+}
+
+#ifdef UNITTEST
+#undef UNITTEST
+#include <assert.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include "mstorage_init.c"
+#include "mstorage_unmap.c"
+
+int main() {
+  mstorage_t m;
+  mstorage_init(&m);
+  // test munmap fail case in mstorage_unmap
+  m.root=(void*)1;
+  m.mapped=4096;
+  assert(mstorage_unmap(&m) == -1);
+  mstorage_init(&m);
+  // regular success cases
+  assert(mstorage_add(&m,"foo",3) == 0);
+  assert(mstorage_add(&m,"bar",3) == 3);
+  assert(mstorage_add(&m,"foo",3) == 6);
+  assert(m.mapped >= 9);
+  assert(!memcmp(m.root,"foobarfoo",9));
+  char* tmp=calloc(PAGE_SIZE,2);
+  assert(mstorage_add(&m,tmp,PAGE_SIZE*2) == 9);
+  // now the failure cases
+  assert(mstorage_unmap(&m) == 0);
+  assert(mstorage_add(&m,"foobarbaz",9) == 0);
+  assert(mstorage_add(&m, "foo", (size_t)-1) == -1);	// #intof2
+  assert(mstorage_add(&m, "foo", (size_t)-1-4096) == -1);	// #intof3
+  assert(mstorage_add(&m, "foo", (size_t)-1-mstorage_increment*2) == -1);	// #inttr2
+  assert(mstorage_add(&m, "foo", ((size_t)-1)/2-mstorage_increment*2) == -1);	// #mmap2
+  assert(mstorage_unmap(&m) == 0);
+  assert(mstorage_add(&m, "foo", (size_t)-1) == -1);	// #intof1
+  assert(mstorage_add(&m, "foo", (size_t)-1-4096) == -1);	// #inttr1
+  assert(mstorage_add(&m, "foo", ((size_t)-1)/2-4096) == -1);	// #mmap1
+  assert(mstorage_unmap(&m) == 0);
+  assert((m.fd=open("Makefile",O_RDONLY)) != -1);
+  assert(mstorage_add(&m, tmp, PAGE_SIZE) == -1);	// #trunc
+  assert(mstorage_unmap(&m) == -1);	// drive-by 100% coverage :)
+  free(tmp);
+
+  return 0;
+}
+#endif

mstorage_add_bin.c

@@ -1,4 +1,6 @@
 #include <libowfat/uint32.h>
+#include <sys/types.h>
+#include <string.h>
 #include "mstorage.h"
 
 /* this is tinyldap specific.  If the data contains at least one 0-byte,
@@ -6,11 +8,38 @@
  *   char 0;
  *   uint32 len;
  *   char data[len] */
-long mstorage_add_bin(mstorage_t* p,const char* s,size_t n) {
-  unsigned int i;
-  static char zero;
-  long x;
-  char intbuf[4];
+ssize_t mstorage_add_bin(mstorage_t* p,const char* s,size_t n) {
+  ssize_t x;
+  if (n >> 31)	// limit length to 32-bit SSIZE_MAX
+    return -1;	// #range
+#if 1
+//  unsigned int i;
+//  static char zero;
+//  char intbuf[4];
+  if (n==0 || memchr(s,0,n)) {
+    // n+5 can't overflow because we limited to SSIZE_MAX on a size_t
+    x=mstorage_reserve(p,n+5);
+    if (x!=-1) {
+      char* t=p->root+x;
+      *t=0;
+      uint32_pack(t+1,n);
+      memcpy(t+5,s,n);
+    }
+    return x;
+  } else {
+    // regular string; make sure there's a 0-terminator
+    // n-1 doesn't underflow because for n==0 we took the other branch
+    size_t needzero=(s[n-1] != 0);
+    x=mstorage_reserve(p,n+needzero);
+    if (x!=-1) {
+      char* t=p->root+x;
+      memcpy(t,s,n);
+      if (needzero)
+	t[n]=0;
+    }
+    return x;
+  }
+#else
   if (n==0 || (n==1 && s[0]==0)) goto encodebinary;
   for (i=0; i<n-1; ++i)
     if (!s[i]) {
@@ -25,4 +54,27 @@ encodebinary:
   if (s[n-1])
     mstorage_add(p,&zero,1);
   return x;
+#endif
 }
+
+#ifdef UNITTEST
+#undef UNITTEST
+#include <assert.h>
+#include "mstorage_add.c"
+#include "mstorage_init.c"
+#include "mstorage_unmap.c"
+int main() {
+  mstorage_t m;
+  mstorage_init(&m);
+  assert(mstorage_add_bin(&m,"foo",(size_t)-1) == -1);	// #range
+  assert(mstorage_add_bin(&m,"foo",3) == 0);
+  // make sure 0 byte is added
+  assert(mstorage_add_bin(&m,"bar",3) == 4);
+  // trigger binary header
+  m.used=0;
+  assert(mstorage_add_bin(&m,"f\x00o",3) == 0);
+  assert(m.used == 8);	// 0, len, f\0o
+  mstorage_unmap(&m);
+  return 0;
+}
+#endif

mstorage_init_persistent.c

@@ -4,17 +4,78 @@
 #include <sys/mman.h>
 #include "mstorage.h"
 
+#ifdef UNITTEST
+#include <errno.h>
+int failafter=0;
+void* my_mmap(void* addr, size_t len, int prot, int flags, int fd, off_t ofs) {
+  if (--failafter == 0) {
+    errno=EINVAL;
+    return MAP_FAILED;
+  }
+  return mmap(addr,len,prot,flags,fd,ofs);
+}
+#define mmap my_mmap
+#endif
+
 int mstorage_init_persistent(mstorage_t* p,int fd) {
   off_t o;
   p->fd=fd;
+  p->mapped=p->used=0;
+  p->root=NULL;
   o=lseek(fd,0,SEEK_END);
-  if (o==-1) return -1;
-  p->mapped=p->used=o;
-  if (p->mapped==0) {
-    p->mapped=4096;
-    if (ftruncate(fd,4096)==-1) return -1;
+  if (o==-1)
+    return -1;
+  if (o==0) {
+    if (ftruncate(fd,4096)==-1)
+      return -1;
+    o=4096;
   }
-  p->root=mmap(0,p->mapped,PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);
-  if (p->root==(char*)-1) return -1;
+  p->root=mmap(0,o,PROT_READ|PROT_WRITE,MAP_SHARED,fd,0);
+  if (p->root==MAP_FAILED) {
+    p->root=NULL;
+    return -1;
+  }
+  p->mapped=p->used=o;
   return 0;
 }
+
+#ifdef UNITTEST
+#include <assert.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <sys/resource.h>
+
+int main() {
+  mstorage_t m;
+  char filename[]="/tmp/fooXXXXXX";
+  int fd;
+  assert((fd=mkstemp(filename)) != -1);
+  assert(unlink(filename) == 0);
+  // success case
+  assert(mstorage_init_persistent(&m, fd) == 0);
+  assert(munmap(m.root, m.mapped) == 0);
+  assert(close(fd) == 0);
+  // make lseek fail
+  int pipes[2];
+  assert(pipe(pipes) == 0);
+  assert(mstorage_init_persistent(&m, pipes[0]) == -1);
+  close(pipes[0]); close(pipes[1]);
+  // make ftruncate fail
+  assert((fd=open(filename, O_RDWR|O_CREAT|O_EXCL, 0600)) != -1);
+  assert(close(fd) == 0);
+  assert((fd=open(filename, O_RDONLY)) != -1);
+  assert(unlink(filename) == 0);
+  // fd is now a read-only handle to a 0 byte file
+  assert(mstorage_init_persistent(&m, fd) == -1);
+  assert(m.root == NULL && m.mapped == 0);
+  assert(close(fd) == 0);
+  // make mmap fail
+  failafter=1;
+  assert((fd=open(filename, O_RDWR|O_CREAT|O_EXCL, 0600)) != -1);
+  assert(unlink(filename) == 0);
+  assert(mstorage_init_persistent(&m, fd) == -1);
+  assert(m.root == NULL && m.mapped == 0);
+  assert(close(fd) == 0);
+  return 0;
+}
+#endif

mstorage_unmap.c

@@ -1,3 +1,6 @@
+/* this is not actually used by tinyldap itself. */
+/* 100% unit test coverage by unit tests in mstorage_add.c */
+
 #include "mstorage.h"
 #include <sys/types.h>
 #include <unistd.h>
@@ -7,16 +10,20 @@
 #include <fcntl.h>
 #include <sys/shm.h>
 
-void mstorage_unmap(mstorage_t* p) {
+int mstorage_unmap(mstorage_t* p) {
+  int r=0;
 #ifdef MREMAP_MAYMOVE
-  munmap(p->root,p->mapped);
+  if (p->root && p->root!=MAP_FAILED)
+    if (munmap(p->root,p->mapped) == -1)
+      r=-1;
 #else
   free(p->root);
 #endif
   if (p->fd!=-1) {
-    ftruncate(p->fd,p->used);
-    close(p->fd);
+    r |= ftruncate(p->fd,p->used);
+    r |= close(p->fd);
   }
   p->mapped=p->used=0;
   p->root=0;
+  return r;
 }