# HG changeset patch # User mas01cr # Date 1227799189 0 # Node ID a65b31660804d37f27d5507a74124dfde12cd070 # Parent 443c2939e84b88b151b59409049a0c9edcaa4c78 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. diff -r 443c2939e84b -r a65b31660804 audioDB.cpp --- 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]; diff -r 443c2939e84b -r a65b31660804 audioDB_API.h --- 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); diff -r 443c2939e84b -r a65b31660804 dump.cpp --- 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; } diff -r 443c2939e84b -r a65b31660804 status.cpp --- 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) {