changeset 460:17003dff8127 api-inversion

Convert index-finding logic to C functions.

New audiodb_index_exists() and audiodb_index_get_name().  The behaviours
of the functions are unchanged, even though they are kind of weird; in
particular, passing in a buffer rather than returning a char * for
audiodb_index_get_name() is probably likely to work better eventually
(see the changes to soap.cpp, which show a clear leak of a buffer in the
error case)

--- a/audioDB-internals.h	Sun Dec 28 22:43:50 2008 +0000
+++ b/audioDB-internals.h	Tue Dec 30 10:36:01 2008 +0000
@@ -224,3 +224,5 @@
 int audiodb_insert_create_datum(adb_insert_t *, adb_datum_t *);
 int audiodb_free_datum(adb_datum_t *);
 int audiodb_query_spec_qpointers(adb_t *, adb_query_spec_t *, double **, double **, adb_qpointers_internal_t *);
+char *audiodb_index_get_name(const char *, double, uint32_t);
+bool audiodb_index_exists(const char *, double, uint32_t);

--- a/audioDB.h	Sun Dec 28 22:43:50 2008 +0000
+++ b/audioDB.h	Tue Dec 30 10:36:01 2008 +0000
@@ -385,7 +385,6 @@
   int index_query_loop(adb_t *adb, adb_query_spec_t *spec, adb_qstate_internal_t *qstate);
   int index_init_query(const char* dbName);
   int index_exists(const char* dbName, double radius, Uns32T sequenceLength);
-  char* index_get_name(const char*dbName, double radius, Uns32T sequenceLength);
   LSH* index_allocate(char* indexName, bool load_hashTables);
   void insertPowerData(unsigned n, int powerfd, double *powerdata);
   void init_track_aux_data(Uns32T trackID, double* fvp, double** sNormpp,double** snPtrp, double** sPowerp, double** spPtrp);

--- a/index.cpp	Sun Dec 28 22:43:50 2008 +0000
+++ b/index.cpp	Tue Dec 30 10:36:01 2008 +0000
@@ -21,28 +21,45 @@
 
 /************************* LSH indexing and query initialization  *****************/
 
-char* audioDB::index_get_name(const char*dbName, double radius, Uns32T sequenceLength){
-  char* indexName = new char[MAXSTR];  
-  // Attempt to make new file
-  if(strlen(dbName) > (MAXSTR - 32))
-    error("dbName is too long for LSH index filename appendages");
+/* FIXME: there are several things wrong with this: the memory
+ * discipline isn't ideal, the radius printing is a bit lame, the name
+ * getting will succeed or fail depending on whether the path was
+ * relative or absolute -- but most importantly encoding all that
+ * information in a filename is going to lose: it's impossible to
+ * maintain backwards-compatibility.  Instead we should probably store
+ * the index metadata inside the audiodb instance. */
+char *audiodb_index_get_name(const char *dbName, double radius, Uns32T sequenceLength) {
+  char *indexName;
+  if(strlen(dbName) > (MAXSTR - 32)) {
+    return NULL;
+  }
+  indexName = new char[MAXSTR];  
   strncpy(indexName, dbName, MAXSTR);
   sprintf(indexName+strlen(dbName), ".lsh.%019.9f.%d", radius, sequenceLength);
   return indexName;
 }
 
-// return true if index exists else return false
-int audioDB::index_exists(const char* dbName, double radius, Uns32T sequenceLength){
-  // Test to see if file exists
-  char* indexName = index_get_name(dbName, radius, sequenceLength);
-  lshfid = open (indexName, O_RDONLY);
-  delete[] indexName;
-  close(lshfid);
-  
-  if(lshfid<0)
-    return false;  
-  else
+bool audiodb_index_exists(const char *dbName, double radius, Uns32T sequenceLength) {
+  char *indexName = audiodb_index_get_name(dbName, radius, sequenceLength);
+  if(!indexName) {
+    return false;
+  }
+  struct stat st;
+  if(stat(indexName, &st)) {
+    delete [] indexName;
+    return false;
+  }
+  /* FIXME: other stat checks here? */
+  /* FIXME: is there any better way to check whether we can open a
+   * file for reading than by opening a file for reading? */
+  int fd = open(indexName, O_RDONLY);
+  delete [] indexName;
+  if(fd < 0) {
+    return false;
+  } else {
+    close(fd);
     return true;
+  }
 }
 
 // If we are a server and have a memory-resident index, check the indexName against the resident index (using get_indexName())
@@ -206,7 +223,10 @@
   if(dbH->flags & O2_FLAG_TIMES)
     usingTimes = true;
 
-  newIndexName = index_get_name(dbName, radius, sequenceLength);
+  newIndexName = audiodb_index_get_name(dbName, radius, sequenceLength);
+  if(!newIndexName) {
+    error("failed to get index name", dbName);
+  }
 
   // Set unit norming flag override
   audioDB::normalizedDistance = !audioDB::no_unit_norming;
@@ -495,10 +515,13 @@
 // return true if indexed query performed else return false
 int audioDB::index_init_query(const char* dbName){
 
-  if(!(index_exists(dbName, radius, sequenceLength)))
+  if(!(audiodb_index_exists(dbName, radius, sequenceLength)))
     return false;
 
-  char* indexName = index_get_name(dbName, radius, sequenceLength);  
+  char *indexName = audiodb_index_get_name(dbName, radius, sequenceLength);
+  if(!indexName) {
+    error("failed to get index name", dbName);
+  }
 
   // Test to see if file exists
   if((lshfid = open (indexName, O_RDONLY)) < 0){
@@ -597,7 +620,10 @@
   if(!index_init_query(adb->path)) // sets-up LSH index structures for querying
     return 0;
 
-  char* database = index_get_name(adb->path, radius, sequenceLength);
+  char *database = audiodb_index_get_name(adb->path, radius, sequenceLength);
+  if(!database) {
+    error("failed to get index name", adb->path);
+  }
 
   if(audiodb_query_spec_qpointers(adb, spec, &query_data, &query, &qpointers)) {
     error("failed to set up qpointers");

--- a/query.cpp	Sun Dec 28 22:43:50 2008 +0000
+++ b/query.cpp	Tue Dec 30 10:36:01 2008 +0000
@@ -146,8 +146,8 @@
     case O2_SEQUENCE_QUERY:
       if(!(qspec.refine.flags & ADB_REFINE_RADIUS)) {
         reporter = new trackAveragingReporter< std::less< NNresult > >(pointNN, trackNN, dbH->numFiles);
-      } else if (index_exists(adb->path, qspec.refine.radius, qspec.qid.sequence_length)) {
-	char* indexName = index_get_name(adb->path, qspec.refine.radius, qspec.qid.sequence_length);
+      } else if (audiodb_index_exists(adb->path, qspec.refine.radius, qspec.qid.sequence_length)) {
+	char *indexName = audiodb_index_get_name(adb->path, qspec.refine.radius, qspec.qid.sequence_length);
 	lsh = index_allocate(indexName, false);
 	reporter = new trackSequenceQueryRadReporter(trackNN, audiodb_index_to_track_id(lsh->get_maxp(), audiodb_lsh_n_point_bits(adb))+1);
 	delete[] indexName;
@@ -158,13 +158,13 @@
     case O2_N_SEQUENCE_QUERY:
       if(!(qspec.refine.flags & ADB_REFINE_RADIUS)) {
         reporter = new trackSequenceQueryNNReporter< std::less < NNresult > >(pointNN, trackNN, dbH->numFiles);
-      } else if (index_exists(adb->path, qspec.refine.radius, qspec.qid.sequence_length)){
-	char* indexName = index_get_name(adb->path, qspec.refine.radius, qspec.qid.sequence_length);
+      } else if (audiodb_index_exists(adb->path, qspec.refine.radius, qspec.qid.sequence_length)){
+	char *indexName = audiodb_index_get_name(adb->path, qspec.refine.radius, qspec.qid.sequence_length);
 	lsh = index_allocate(indexName, false);
-	reporter = new trackSequenceQueryRadNNReporter(pointNN,trackNN, audiodb_index_to_track_id(lsh->get_maxp(), audiodb_lsh_n_point_bits(adb))+1);
+	reporter = new trackSequenceQueryRadNNReporter(pointNN, trackNN, audiodb_index_to_track_id(lsh->get_maxp(), audiodb_lsh_n_point_bits(adb))+1);
 	delete[] indexName;
       } else {
-	reporter = new trackSequenceQueryRadNNReporter(pointNN,trackNN, dbH->numFiles);
+	reporter = new trackSequenceQueryRadNNReporter(pointNN, trackNN, dbH->numFiles);
       }
       break;
     }
@@ -236,7 +236,7 @@
   }
   
   // Test for index (again) here
-  if((qspec.refine.flags & ADB_REFINE_RADIUS) && index_exists(adb->path, qspec.refine.radius, qspec.qid.sequence_length)){ 
+  if((qspec.refine.flags & ADB_REFINE_RADIUS) && audiodb_index_exists(adb->path, qspec.refine.radius, qspec.qid.sequence_length)){ 
     VERB_LOG(1, "Calling indexed query on database %s, radius=%f, sequence_length=%d\n", adb->path, qspec.refine.radius, qspec.qid.sequence_length);
     index_query_loop(adb, &qspec, &qstate);
   }

--- a/soap.cpp	Sun Dec 28 22:43:50 2008 +0000
+++ b/soap.cpp	Tue Dec 30 10:36:01 2008 +0000
@@ -1,4 +1,5 @@
 #include "audioDB.h"
+#include "audioDB-internals.h"
 #include "adb.nsmap"
 
 /* Command-line client definitions */
@@ -446,11 +447,12 @@
     {
       fprintf(stderr, "Socket connection successful: master socket = %d\n", m);
       // Make a global Web Services LSH Index (SINGLETON)
-      if(WS_load_index && dbName && !index_exists(dbName, radius, sequenceLength)){
-        error("Can't find requested index file:", index_get_name(dbName,radius,sequenceLength));
+      if(WS_load_index && dbName && !audiodb_index_exists(dbName, radius, sequenceLength)){
+        /* FIXME: this leaks the indexName */
+        error("Can't find requested index file:", audiodb_index_get_name(dbName,radius,sequenceLength));
       }
-      if(WS_load_index && dbName && index_exists(dbName, radius, sequenceLength)){
-	char* indexName = index_get_name(dbName, radius, sequenceLength);
+      if(WS_load_index && dbName && audiodb_index_exists(dbName, radius, sequenceLength)){
+	char *indexName = audiodb_index_get_name(dbName, radius, sequenceLength);
 	fprintf(stderr, "Loading LSH hashtables: %s...\n", indexName);
 	lsh = new LSH(indexName, true);
 	assert(lsh);