changeset 399:a65b31660804 api-inversion

Invert audioDB::dump / audiodb_dump().

No real API/ABI breakages, modulo the disappearance of
audiodb_dump_withdir() (which really should have been audiodb_dump()
itself from the start).  There were of course ABI breakages in the
previous commits.

The dodgy thing in this patch is the horribleness of audiodb_dump()
itself; there must be a better way of writing it, or at least
abstracting some of the body into individual functional pieces.  The
declaration block at the top tells its own story.

We also need to alter the way that audioDB::status handles the adb;
rather than having a local variable, use the C++ audioDB object instance
field and only open the database if necessary -- then everything has a
consistent view.

--- a/audioDB.cpp	Thu Nov 27 15:19:47 2008 +0000
+++ b/audioDB.cpp	Thu Nov 27 15:19:49 2008 +0000
@@ -706,15 +706,15 @@
 }
 
 void audioDB::status(const char* dbName, adb__statusResponse *adbStatusResponse){
-  adb_t *adb;
   adb_status_t status;
-  if(!(adb = audiodb_open(dbName, O_RDONLY))) {
-    error("Failed to open database file", dbName);
+  if(!adb) {
+    if(!(adb = audiodb_open(dbName, O_RDONLY))) {
+      error("Failed to open database file", dbName);
+    }
   }
   if(audiodb_status(adb, &status)) {
     error("Failed to retrieve database status", dbName);
   }
-  audiodb_close(adb);
   
   if(adbStatusResponse == 0) {
     std::cout << "num files:" << status.numFiles << std::endl;
@@ -781,13 +781,25 @@
 }
 
 void audioDB::create(const char *dbName) {
-  adb_t *adb;
+  if(adb) {
+    error("Already have an adb in this object", "");
+  }
   if(!(adb = audiodb_create(dbName, datasize, ntracks, datadim))) {
     error("Failed to create database file", dbName);
   }
-  audiodb_close(adb);
 }
 
+void audioDB::dump(const char *dbName) {
+  if(!adb) {
+    if(!(adb = audiodb_open(dbName, O_RDONLY))) {
+      error("Failed to open database file", dbName);
+    }
+  }
+  if(audiodb_dump(adb, output)) {
+    error("Failed to dump database to ", output);
+  }
+  status(dbName);
+}
 // Unit norm block of features
 
 /* FIXME: in fact this does not unit norm a block of features, it just
@@ -1128,29 +1140,6 @@
     return apierror;
   }
 
-  int audiodb_dump(adb_ptr mydb){
-      return audiodb_dump_withdir(mydb,"audioDB.dump");
-  }
-
-  int audiodb_dump_withdir(adb_ptr mydb, const char *outputdir){
-
-      const char *argv[7];
-      int argvctr=0;
-      int apierror=0;
-
-      argv[argvctr++]="audioDB";
-      argv[argvctr++]="--DUMP";
-      argv[argvctr++]="-d";
-      argv[argvctr++]=mydb->path;
-      argv[argvctr++]="--output";
-      argv[argvctr++]=(char *)outputdir;
-      argv[argvctr]='\0';
-
-      audioDB::audioDB(6,argv,&apierror,mydb);
-
-      return apierror;
-  }
-
   int audiodb_l2norm(adb_ptr mydb){
 
       const char *argv[5];

--- a/audioDB_API.h	Thu Nov 27 15:19:47 2008 +0000
+++ b/audioDB_API.h	Thu Nov 27 15:19:49 2008 +0000
@@ -118,7 +118,6 @@
 int audiodb_status(adb_ptr mydb, adb_status_ptr status);
 
 /* varoius dump formats */
-int audiodb_dump(adb_ptr mydb);
-int audiodb_dump_withdir(adb_ptr mydb, const char *outputdir);
+int audiodb_dump(adb_ptr mydb, const char *outputdir);
 
 

--- a/dump.cpp	Thu Nov 27 15:19:47 2008 +0000
+++ b/dump.cpp	Thu Nov 27 15:19:49 2008 +0000
@@ -1,47 +1,115 @@
 #include "audioDB.h"
+extern "C" {
+#include "audioDB_API.h"
+}
 
-void audioDB::dump(const char* dbName){
-  if(!dbH) {
-    initTables(dbName, 0);
+/* We could go gcc-specific here and use typeof() instead of passing
+ * in an explicit type.  Answers on a postcard as to whether that's a
+ * good plan or not. */
+#define mmap_or_goto_error(type, var, start, length) \
+  { void *tmp = mmap(0, length, PROT_READ, MAP_SHARED, adb->fd, (start)); \
+    if(tmp == (void *) -1) { \
+      goto error; \
+    } \
+    var = (type) tmp; \
+  }
+
+#define maybe_munmap(table, length) \
+  { if(table) { \
+      munmap(table, length); \
+    } \
+  }
+
+int audiodb_dump(adb_t *adb, const char *output) {
+  char *fileTable = 0; /* key_table */
+  unsigned *trackTable = 0; /* track_size_table */
+  double *timesTable = 0; /* timestamps_table */
+  double *powerTable = 0; /* power_table */
+
+  size_t fileTableLength = 0;
+  size_t trackTableLength = 0;
+  size_t timesTableLength = 0;
+  size_t powerTableLength = 0;
+
+  char *featureFileNameTable = 0;
+  char *powerFileNameTable = 0;
+  char *timesFileNameTable = 0;
+ 
+  char cwd[PATH_MAX];
+  int directory_changed = 0;
+
+  int fLfd = 0, tLfd = 0, pLfd = 0, kLfd = 0;
+  FILE *fLFile = 0, *tLFile = 0, *pLFile = 0, *kLFile = 0;
+
+  int times, power;
+
+  char fName[256];
+  int ffd, pfd;
+  FILE *tFile;
+  unsigned pos = 0;
+  double *data_buffer;
+  size_t data_buffer_size;
+  FILE *scriptFile = 0;
+
+  unsigned nfiles = adb->header->numFiles;
+
+  if(adb->header->length > 0) {
+    fileTableLength = ALIGN_PAGE_UP(nfiles * O2_FILETABLE_ENTRY_SIZE);
+    trackTableLength = ALIGN_PAGE_UP(nfiles * O2_TRACKTABLE_ENTRY_SIZE);
+    if(!(adb->header->flags & O2_FLAG_LARGE_ADB)) {
+      off_t length = adb->header->length;
+      unsigned dim = adb->header->dim;
+      timesTableLength = ALIGN_PAGE_UP(2*length/dim);
+      powerTableLength = ALIGN_PAGE_UP(length/dim);
+    }
+
+    mmap_or_goto_error(char *, fileTable, adb->header->fileTableOffset, fileTableLength);
+    mmap_or_goto_error(unsigned *, trackTable, adb->header->trackTableOffset, trackTableLength);
+    if (adb->header->flags & O2_FLAG_LARGE_ADB) {
+      mmap_or_goto_error(char *, featureFileNameTable, adb->header->dataOffset, fileTableLength);
+      mmap_or_goto_error(char *, powerFileNameTable, adb->header->powerTableOffset, fileTableLength);
+      mmap_or_goto_error(char *, timesFileNameTable, adb->header->timesTableOffset, fileTableLength);
+    } else {
+      mmap_or_goto_error(double *, powerTable, adb->header->powerTableOffset, powerTableLength);
+      mmap_or_goto_error(double *, timesTable, adb->header->timesTableOffset, timesTableLength);
+    }
   }
 
   if((mkdir(output, S_IRWXU|S_IRWXG|S_IRWXO)) < 0) {
-    error("error making output directory", output, "mkdir");
+    goto error;
   }
 
-  char *cwd = new char[PATH_MAX];
-
   if ((getcwd(cwd, PATH_MAX)) == 0) {
-    error("error getting working directory", "", "getcwd");
+    goto error;
   }
 
+  /* FIXME: Hrm.  How does chdir(2) interact with threads?  Does each
+   * thread have its own working directory? */
   if((chdir(output)) < 0) {
-    error("error changing working directory", output, "chdir");
+    goto error;
+  }
+  directory_changed = 1;
+
+  if ((fLfd = open("featureList.txt", O_CREAT|O_RDWR|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) {
+    goto error;
   }
 
-  int fLfd, tLfd = 0, pLfd = 0, kLfd;
-  FILE *fLFile, *tLFile = 0, *pLFile = 0, *kLFile;
-
-  if ((fLfd = open("featureList.txt", O_CREAT|O_RDWR|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) {
-    error("error creating featureList file", "featureList.txt", "open");
-  }
-
-  int times = dbH->flags & O2_FLAG_TIMES;
+  times = adb->header->flags & O2_FLAG_TIMES;
   if (times) {
     if ((tLfd = open("timesList.txt", O_CREAT|O_RDWR|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) {
-      error("error creating timesList file", "timesList.txt", "open");
+      goto error;
     }
   }
 
-  int power = dbH->flags & O2_FLAG_POWER;
+  power = adb->header->flags & O2_FLAG_POWER;
   if (power) {
     if ((pLfd = open("powerList.txt", O_CREAT|O_RDWR|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) {
-      error("error creating powerList file", "powerList.txt", "open");
+      goto error;
     }
   }
 
   if ((kLfd = open("keyList.txt", O_CREAT|O_RDWR|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) {
-    error("error creating keyList file", "keyList.txt", "open");
+    goto error;
   }
   
   /* can these fail?  I sincerely hope not. */
@@ -54,69 +122,65 @@
   }
   kLFile = fdopen(kLfd, "w");
 
-  char *fName = new char[256];
-  int ffd, pfd;
-  FILE *tFile;
-  unsigned pos = 0;
-  lseek(dbfid, dbH->dataOffset, SEEK_SET);
-  double *data_buffer;
-  size_t data_buffer_size;
-  for(unsigned k = 0; k < dbH->numFiles; k++) {
+  lseek(adb->fd, adb->header->dataOffset, SEEK_SET);
+
+  for(unsigned k = 0; k < nfiles; k++) {
     fprintf(kLFile, "%s\n", fileTable + k*O2_FILETABLE_ENTRY_SIZE);
-    if(dbH->flags & O2_FLAG_LARGE_ADB) {
+    if(adb->header->flags & O2_FLAG_LARGE_ADB) {
       char *featureFileName = featureFileNameTable+k*O2_FILETABLE_ENTRY_SIZE;
+      if(*featureFileName != '/') {
+        goto error;
+      }
       fprintf(fLFile, "%s\n", featureFileName);
-      if(*featureFileName != '/') {
-	error("relative path in LARGE_ADB", featureFileName);
-      }
       if(times) {
 	char *timesFileName = timesFileNameTable + k*O2_FILETABLE_ENTRY_SIZE;
+	if(*timesFileName != '/') {
+          goto error;
+	}
 	fprintf(tLFile, "%s\n", timesFileName);
-	if(*timesFileName != '/') {
-	  error("relative path in LARGE_ADB", timesFileName);	  
-	}
       }
       if(power) {
 	char *powerFileName = powerFileNameTable + k*O2_FILETABLE_ENTRY_SIZE;
+	if(*powerFileName != '/') {
+          goto error;
+	}
 	fprintf(pLFile, "%s\n", powerFileName);
-	if(*powerFileName != '/') {
-	  error("relative path in LARGE_ADB", powerFileName);
-	}
       }
     } else {
       snprintf(fName, 256, "%05d.features", k);
       if ((ffd = open(fName, O_CREAT|O_RDWR|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) {
-	error("error creating feature file", fName, "open");
+        goto error;
       }
-      if ((write(ffd, &dbH->dim, sizeof(uint32_t))) < 0) {
-	error("error writing dimensions", fName, "write");
+      if ((write(ffd, &(adb->header->dim), sizeof(uint32_t))) < 0) {
+        goto error;
       }
       
       /* FIXME: this repeated malloc()/free() of data buffers is
 	 inefficient. */
-      data_buffer_size = trackTable[k] * dbH->dim * sizeof(double);
+      data_buffer_size = trackTable[k] * adb->header->dim * sizeof(double);
       
       {
 	void *tmp = malloc(data_buffer_size);
 	if (tmp == NULL) {
-	  error("error allocating data buffer");
+          goto error;
 	}
 	data_buffer = (double *) tmp;
       }
       
-      if ((read(dbfid, data_buffer, data_buffer_size)) != (ssize_t) data_buffer_size) {
-	error("error reading data", fName, "read");
+      if ((read(adb->fd, data_buffer, data_buffer_size)) != (ssize_t) data_buffer_size) {
+        goto error;
       }
       
       if ((write(ffd, data_buffer, data_buffer_size)) < 0) {
-	error("error writing data", fName, "write");
+        goto error;
       }
       
       free(data_buffer);
       
       fprintf(fLFile, "%s\n", fName);
       close(ffd);
-      
+      ffd = 0;
+
       if (times) {
 	snprintf(fName, 256, "%05d.times", k);
 	tFile = fopen(fName, "w");
@@ -130,6 +194,7 @@
 	  fprintf(tFile, "%.16e\n", *(timesTable + 2*pos + 2*i));
 	}
 	fprintf(tFile, "%.16e\n", *(timesTable + 2*pos + 2*trackTable[k]-1));
+        fclose(tFile);
 	
 	fprintf(tLFile, "%s\n", fName);
       }
@@ -138,16 +203,17 @@
 	uint32_t one = 1;
 	snprintf(fName, 256, "%05d.power", k);
 	if ((pfd = open(fName, O_CREAT|O_RDWR|O_EXCL, S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH)) < 0) {
-	  error("error creating power file", fName, "open");
+          goto error;
 	}
 	if ((write(pfd, &one, sizeof(uint32_t))) < 0) {
-	  error("error writing one", fName, "write");
+          goto error;
 	}
 	if ((write(pfd, powerTable + pos, trackTable[k] * sizeof(double))) < 0) {
-	  error("error writing data", fName, "write");
+          goto error;
 	}
 	fprintf(pLFile, "%s\n", fName);
 	close(pfd);
+        pfd = 0;
       } 
       
       pos += trackTable[k];
@@ -155,7 +221,6 @@
     }
   }
 
-  FILE *scriptFile;
   scriptFile = fopen("restore.sh", "w");
   fprintf(scriptFile, "\
 #! /bin/sh\n\
@@ -165,12 +230,12 @@
 if [ -z \"${AUDIODB}\" ]; then echo set AUDIODB variable; exit 1; fi\n\
 if [ -z \"$1\" ]; then echo usage: $0 newdb; exit 1; fi\n\n\
 \"${AUDIODB}\" -d \"$1\" -N --datasize=%d --ntracks=%d --datadim=%d\n",
-          (int) ((dbH->timesTableOffset - dbH->dataOffset) / (1024*1024)),
+          (int) ((adb->header->timesTableOffset - adb->header->dataOffset) / (1024*1024)),
           // fileTable entries (char[256]) are bigger than trackTable
           // (int), so the granularity of page aligning is finer.
-          (int) ((dbH->trackTableOffset - dbH->fileTableOffset) / O2_FILETABLE_ENTRY_SIZE),
-          (int) ceil(((double) (dbH->timesTableOffset - dbH->dataOffset)) / ((double) (dbH->dbSize - dbH->l2normTableOffset))));
-  if(dbH->flags & O2_FLAG_L2NORM) {
+          (int) ((adb->header->trackTableOffset - adb->header->fileTableOffset) / O2_FILETABLE_ENTRY_SIZE),
+          (int) ceil(((double) (adb->header->timesTableOffset - adb->header->dataOffset)) / ((double) (adb->header->dbSize - adb->header->l2normTableOffset))));
+  if(adb->header->flags & O2_FLAG_L2NORM) {
     fprintf(scriptFile, "\"${AUDIODB}\" -d \"$1\" -L\n");
   }
   if(power) {
@@ -186,10 +251,6 @@
   fprintf(scriptFile, "\n");
   fclose(scriptFile);
 
-  if((chdir(cwd)) < 0) {
-    error("error changing working directory", cwd, "chdir");
-  }
-
   fclose(fLFile);
   if(times) {
     fclose(tLFile);
@@ -198,7 +259,58 @@
     fclose(pLFile);
   }
   fclose(kLFile);
-  delete[] fName;
     
-  status(dbName);
+  maybe_munmap(fileTable, fileTableLength);
+  maybe_munmap(trackTable, trackTableLength);
+  maybe_munmap(timesTable, timesTableLength);
+  maybe_munmap(powerTable, powerTableLength);
+  maybe_munmap(featureFileNameTable, fileTableLength);
+  maybe_munmap(timesFileNameTable, fileTableLength);
+  maybe_munmap(powerFileNameTable, fileTableLength);
+
+  if((chdir(cwd)) < 0) {
+    /* don't goto error because the error handling will try to
+     * chdir() */
+    return 1;
+  }
+
+  return 0;
+
+ error:
+  if(fLFile) {
+    fclose(fLFile);
+  } else if(fLfd) {
+    close(fLfd);
+  }
+  if(tLFile) {
+    fclose(tLFile);
+  } else if(tLfd) {
+    close(fLfd);
+  }
+  if(pLFile) {
+    fclose(pLFile);
+  } else if(pLfd) {
+    close(pLfd);
+  }
+  if(kLFile) {
+    fclose(kLFile);
+  } else if(kLfd) {
+    close(kLfd);
+  }
+  if(scriptFile) {
+    fclose(scriptFile);
+  }
+
+  maybe_munmap(fileTable, fileTableLength);
+  maybe_munmap(trackTable, trackTableLength);
+  maybe_munmap(timesTable, timesTableLength);
+  maybe_munmap(powerTable, powerTableLength);
+  maybe_munmap(featureFileNameTable, fileTableLength);
+  maybe_munmap(timesFileNameTable, fileTableLength);
+  maybe_munmap(powerFileNameTable, fileTableLength);
+
+  if(directory_changed) {
+    chdir(cwd);
+  }
+  return 1;
 }

--- a/status.cpp	Thu Nov 27 15:19:47 2008 +0000
+++ b/status.cpp	Thu Nov 27 15:19:49 2008 +0000
@@ -11,7 +11,7 @@
   unsigned dudCount = 0;
   unsigned nullCount = 0;
 
-  off_t trackTableLength = ALIGN_PAGE_UP(adb->header->numFiles * O2_TRACKTABLE_ENTRY_SIZE);
+  size_t trackTableLength = ALIGN_PAGE_UP(adb->header->numFiles * O2_TRACKTABLE_ENTRY_SIZE);
   unsigned *trackTable = 0;
   void *tmp = 0;
   if (adb->header->length > 0)  {