From f85f30c3798f26e4ca8d270b4a444a9c9dc9e7b6 Mon Sep 17 00:00:00 2001 From: leitner Date: Fri, 2 Feb 2024 14:38:25 +0000 Subject: [PATCH] add unit tests --- mduptab.h | 13 ++++- mduptab_add.c | 48 +++++++++++++++- mduptab_init_reuse.c | 2 + mstorage.h | 13 +++-- mstorage_add.c | 113 +++++++++++++++++++++++++++++-------- mstorage_add_bin.c | 62 ++++++++++++++++++-- mstorage_init_persistent.c | 75 +++++++++++++++++++++--- mstorage_unmap.c | 15 +++-- 8 files changed, 294 insertions(+), 47 deletions(-) diff --git a/mduptab.h b/mduptab.h index 5704e87..a37aa90 100644 --- a/mduptab.h +++ b/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 diff --git a/mduptab_add.c b/mduptab_add.c index 8705c6b..c4275c7 100644 --- a/mduptab_add.c +++ b/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 #include #include @@ -6,18 +11,55 @@ #include "mduptab.h" #include -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; itable.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 +#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 diff --git a/mduptab_init_reuse.c b/mduptab_init_reuse.c index ea22404..c3f50f1 100644 --- a/mduptab_init_reuse.c +++ b/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) { diff --git a/mstorage.h b/mstorage.h index 258e786..e1c5204 100644 --- a/mstorage.h +++ b/mstorage.h @@ -2,6 +2,7 @@ #define _MSTORAGE_H #include +#include /* (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 diff --git a/mstorage_add.c b/mstorage_add.c index ada0594..80d40f6 100644 --- a/mstorage_add.c +++ b/mstorage_add.c @@ -5,7 +5,7 @@ #include #include #include -#include +#include #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->usedroot) { /* 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 +#include +#include +#include +#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 diff --git a/mstorage_add_bin.c b/mstorage_add_bin.c index 322f1ca..f99c018 100644 --- a/mstorage_add_bin.c +++ b/mstorage_add_bin.c @@ -1,4 +1,6 @@ #include +#include +#include #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 +#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 diff --git a/mstorage_init_persistent.c b/mstorage_init_persistent.c index 1d6ac09..1f3fa6d 100644 --- a/mstorage_init_persistent.c +++ b/mstorage_init_persistent.c @@ -4,17 +4,78 @@ #include #include "mstorage.h" +#ifdef UNITTEST +#include +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 +#include +#include +#include + +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 diff --git a/mstorage_unmap.c b/mstorage_unmap.c index 32b66a4..2201700 100644 --- a/mstorage_unmap.c +++ b/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 #include @@ -7,16 +10,20 @@ #include #include -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; }