[27528] trunk/base/src

source_changes at macosforge.org source_changes at macosforge.org
Mon Aug 6 13:13:31 PDT 2007


Revision: 27528
          http://trac.macosforge.org/projects/macports/changeset/27528
Author:   sfiera at macports.org
Date:     2007-08-06 13:13:30 -0700 (Mon, 06 Aug 2007)

Log Message:
-----------
Better error codes

Uses Tcl hashes instead of temporary tables
Plenty of memory leaks fixed
Handles SQLITE_BUSY properly
No longer forces an explicit write-transaction
Removed reg parameter when unnecessary
Deal with reverted deletions properly
Lots of documentation
More?

Modified Paths:
--------------
    trunk/base/src/cregistry/entry.c
    trunk/base/src/cregistry/entry.h
    trunk/base/src/cregistry/registry.c
    trunk/base/src/cregistry/registry.h
    trunk/base/src/cregistry/sql.c
    trunk/base/src/registry2.0/entry.c
    trunk/base/src/registry2.0/entry.h
    trunk/base/src/registry2.0/entryobj.c
    trunk/base/src/registry2.0/registry.c
    trunk/base/src/registry2.0/registry.h
    trunk/base/src/registry2.0/tests/entry.tcl
    trunk/base/src/registry2.0/util.c

Modified: trunk/base/src/cregistry/entry.c
===================================================================
--- trunk/base/src/cregistry/entry.c	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/cregistry/entry.c	2007-08-06 20:13:30 UTC (rev 27528)
@@ -38,6 +38,23 @@
 #include <cregistry/registry.h>
 #include <cregistry/sql.h>
 
+/*
+ * TODO: possibly, allow reg_entry_search to take different matching strategies
+ *       for different keys. I don't know of an application for this feature
+ *       yet, so no reason to bother for now.
+ *
+ * TODO: reg_entry_installed and reg_entry_imaged could benefit from the added
+ *       flexibility of -glob and -regexp too. Not a high priority, though.
+ *
+ * TODO: process the 'state' keyword in a more efficient way. I still believe
+ *       there could be benefits to be reaped in the future by allowing
+ *       arbitrary values but at the same time it will always have very discrete
+ *       values. These could be more efficiently dealt with as integers.
+ *
+ * TODO: move the utility functions to util.h or something. Not important until
+ *       there are more types in the registry than entry, though.
+ */
+
 /**
  * Concatenates `src` to string `dst`. Simple concatenation. Only guaranteed to
  * work with strings that have been allocated with `malloc`. Amortizes cost of
@@ -88,22 +105,23 @@
 }
 
 /**
- * Returns the operator to use for the given strategy.
+ * Returns an expression to use for the given strategy. This should be passed as
+ * the `fmt` argument of `sqlite3_mprintf`, with the key and value following.
  *
  * @param [in] strategy a strategy (one of the `reg_strategy_*` constants)
  * @param [out] errPtr  on error, a description of the error that occurred
- * @return              a sqlite3 operator if successful; NULL otherwise
+ * @return              a sqlite3 expression if success; NULL if failure
  */
 static char* reg_strategy_op(reg_strategy strategy, reg_error* errPtr) {
     switch (strategy) {
-        case reg_strategy_equal:
-            return "=";
+        case reg_strategy_exact:
+            return "%q = '%q'";
         case reg_strategy_glob:
-            return " GLOB ";
+            return "%q GLOB '%q'";
         case reg_strategy_regexp:
-            return " REGEXP ";
+            return "REGEXP(%q, '%q')";
         default:
-            errPtr->code = "registry::invalid";
+            errPtr->code = REG_INVALID;
             errPtr->description = "invalid matching strategy specified";
             errPtr->free = NULL;
             return NULL;
@@ -123,83 +141,45 @@
  */
 static int reg_stmt_to_entry(void* userdata, void** entry, void* stmt,
         reg_error* errPtr UNUSED) {
-    if (sqlite3_column_type(stmt, 1) == SQLITE_NULL) {
+    int is_new;
+    reg_registry* reg = (reg_registry*)userdata;
+    sqlite_int64 id = sqlite3_column_int64(stmt, 0);
+    Tcl_HashEntry* hash = Tcl_CreateHashEntry(&reg->open_entries,
+            (const char*)&id, &is_new);
+    if (is_new) {
         reg_entry* e = malloc(sizeof(reg_entry));
-        e->db = (sqlite3*)userdata;
-        e->id = sqlite3_column_int64(stmt, 0);
-        e->saved = 0;
+        e->reg = reg;
+        e->id = id;
         e->proc = NULL;
         *entry = e;
+        Tcl_SetHashValue(hash, e);
     } else {
-        *entry = *(reg_entry**)sqlite3_column_blob(stmt, 1);
+        *entry = Tcl_GetHashValue(hash);
     }
     return 1;
 }
 
 /**
- * Saves the addresses of existing `reg_entry` items into the temporary sqlite3
- * table `entries`. These addresses will be retrieved by anything else that
- * needs to get entries, so only one `reg_entry` will exist in memory for any
- * given id. They will be freed when the registry is closed.
+ * Creates a new entry in the ports registry. Unlike the old
+ * `registry::new_entry`, revision, variants, and epoch are all required. That's
+ * OK because there's only one place this function is called, and it's called
+ * with all of them there.
  *
- * @param [in] db          sqlite3 database to save entries into
- * @param [in] entries     list of entries to save
- * @param [in] entry_count number of entries to save
- * @param [out] errPtr     on error, a description of the error that occurred
+ * @param [in] reg      the registry to create the entry in
+ * @param [in] name     name of port
+ * @param [in] version  version of port
+ * @param [in] revision revision of port
+ * @param [in] variants variants to record
+ * @param [in] epoch    epoch of port
+ * @param [out] errPtr  on error, a description of the error that occurred
+ * @return              the entry if success; NULL if failure
  */
-static int reg_save_addresses(sqlite3* db, reg_entry** entries,
-        int entry_count, reg_error* errPtr) {
-    sqlite3_stmt* stmt;
-    int i;
-    char* query = "INSERT INTO entries (id, address) VALUES (?, ?)";
-    /* avoid preparing the statement if unnecessary */
-    for (i=0; i<entry_count; i++) {
-        if (!entries[i]->saved) {
-            break;
-        }
-    }
-    if (i == entry_count) {
-        return 1;
-    }
-    if (sqlite3_prepare(db, query, -1, &stmt, NULL)
-                == SQLITE_OK) {
-        for (i=0; i<entry_count; i++) {
-            if (entries[i]->saved) {
-                continue;
-            }
-            if ((sqlite3_bind_int64(stmt, 1, entries[i]->id) == SQLITE_OK)
-                    && (sqlite3_bind_blob(stmt, 2, &entries[i],
-                            sizeof(reg_entry*), SQLITE_TRANSIENT) == SQLITE_OK)
-                    && (sqlite3_step(stmt) == SQLITE_DONE)) {
-                sqlite3_reset(stmt);
-            } else {
-                sqlite3_finalize(stmt);
-                reg_sqlite_error(db, errPtr, query);
-                return 0;
-            }
-        }
-        sqlite3_finalize(stmt);
-        return 1;
-    } else {
-        sqlite3_finalize(stmt);
-        reg_sqlite_error(db, errPtr, query);
-    }
-    return 0;
-}
-
-/**
- * Unlike the old registry::new_entry, revision, variants, and epoch are all
- * required. That's OK because there's only one place this function is called,
- * and it's called with all of them there.
- */
 reg_entry* reg_entry_create(reg_registry* reg, char* name, char* version,
         char* revision, char* variants, char* epoch, reg_error* errPtr) {
     sqlite3_stmt* stmt;
+    reg_entry* entry = NULL;
     char* query = "INSERT INTO registry.ports "
         "(name, version, revision, variants, epoch) VALUES (?, ?, ?, ?, ?)";
-    if (!reg_test_writable(reg, errPtr)) {
-        return NULL;
-    }
     if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
             && (sqlite3_bind_text(stmt, 1, name, -1, SQLITE_STATIC)
                 == SQLITE_OK)
@@ -210,40 +190,53 @@
             && (sqlite3_bind_text(stmt, 4, variants, -1, SQLITE_STATIC)
                 == SQLITE_OK)
             && (sqlite3_bind_text(stmt, 5, epoch, -1, SQLITE_STATIC)
-                == SQLITE_OK)
-            && (sqlite3_step(stmt) == SQLITE_DONE)) {
-        char* query = "INSERT INTO entries (id, address) VALUES (?, ?)";
-        reg_entry* entry = malloc(sizeof(reg_entry));
-        entry->id = sqlite3_last_insert_rowid(reg->db);
-        entry->db = reg->db;
-        entry->proc = NULL;
-        sqlite3_finalize(stmt);
-        if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL)
-                == SQLITE_OK)
-                && (sqlite3_bind_int64(stmt, 1, entry->id) == SQLITE_OK)
-                && (sqlite3_bind_blob(stmt, 2, &entry, sizeof(reg_entry*),
-                        SQLITE_TRANSIENT) == SQLITE_OK)
-                && (sqlite3_step(stmt) == SQLITE_DONE)) {
-            return entry;
-        } else {
-            reg_sqlite_error(reg->db, errPtr, query);
-        }
-        free(entry);
-        return NULL;
+                == SQLITE_OK)) {
+        int r;
+        do {
+            Tcl_HashEntry* hash;
+            int is_new;
+            r = sqlite3_step(stmt);
+            switch (r) {
+                case SQLITE_DONE:
+                    entry = malloc(sizeof(reg_entry));
+                    entry->id = sqlite3_last_insert_rowid(reg->db);
+                    entry->reg = reg;
+                    entry->proc = NULL;
+                    hash = Tcl_CreateHashEntry(&reg->open_entries,
+                            (const char*)&entry->id, &is_new);
+                    Tcl_SetHashValue(hash, entry);
+                    break;
+                case SQLITE_BUSY:
+                    break;
+                default:
+                    reg_sqlite_error(reg->db, errPtr, query);
+                    break;
+            }
+        } while (r == SQLITE_BUSY);
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
-        sqlite3_finalize(stmt);
-        return NULL;
     }
+    sqlite3_finalize(stmt);
+    return entry;
 }
 
 /**
+ * Opens an existing entry in the registry.
+ *
+ * @param [in] reg      registry to open entry in
+ * @param [in] name     name of port
+ * @param [in] version  version of port
+ * @param [in] revision revision of port
+ * @param [in] variants variants to record
+ * @param [in] epoch    epoch of port
+ * @param [out] errPtr  on error, a description of the error that occurred
+ * @return              the entry if success; NULL if failure
  */
 reg_entry* reg_entry_open(reg_registry* reg, char* name, char* version,
         char* revision, char* variants, char* epoch, reg_error* errPtr) {
     sqlite3_stmt* stmt;
-    char* query = "SELECT registry.ports.id, entries.address "
-        "FROM registry.ports LEFT OUTER JOIN entries USING (id) "
+    reg_entry* entry = NULL;
+    char* query = "SELECT registry.ports.id FROM registry.ports "
         "WHERE name=? AND version=? AND revision=? AND variants=? AND epoch=?";
     if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
             && (sqlite3_bind_text(stmt, 1, name, -1, SQLITE_STATIC)
@@ -256,147 +249,191 @@
                 == SQLITE_OK)
             && (sqlite3_bind_text(stmt, 5, epoch, -1, SQLITE_STATIC)
                 == SQLITE_OK)) {
-        int r = sqlite3_step(stmt);
-        reg_entry* entry;
-        switch (r) {
-            case SQLITE_ROW:
-                if (reg_stmt_to_entry(reg->db, (void**)&entry, stmt, errPtr)) {
-                    sqlite3_finalize(stmt);
-                    if (reg_save_addresses(reg->db, &entry, 1, errPtr)) {
-                        return entry;
-                    }
-                }
-            case SQLITE_DONE:
-                errPtr->code = "registry::not-found";
-                errPtr->description = "no matching port found";
-                errPtr->free = NULL;
-                break;
-            default:
-                reg_sqlite_error(reg->db, errPtr, query);
-                break;
-        }
-        sqlite3_finalize(stmt);
-        return NULL;
+        int r;
+        do {
+            r = sqlite3_step(stmt);
+            switch (r) {
+                case SQLITE_ROW:
+                    reg_stmt_to_entry(reg, (void**)&entry, stmt, errPtr);
+                    break;
+                case SQLITE_DONE:
+                    errPtr->code = REG_NOT_FOUND;
+                    errPtr->description = "no matching port found";
+                    errPtr->free = NULL;
+                    break;
+                case SQLITE_BUSY:
+                    continue;
+                default:
+                    reg_sqlite_error(reg->db, errPtr, query);
+                    break;
+            }
+        } while (r == SQLITE_BUSY);
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
-        sqlite3_finalize(stmt);
-        return NULL;
     }
+    sqlite3_finalize(stmt);
+    return entry;
 }
 
 /**
- * Deletes and frees an entry.
+ * Deletes an entry. After calling this, `reg_entry_free` needs to be called
+ * manually on the entry. Care should be taken to not free the entry if this
+ * deletion is rolled back.
+ *
+ * @param [in] entry   the entry to delete
+ * @param [out] errPtr on error, a description of the error that occurred
+ * @return             true if success; false if failure
  */
-int reg_entry_delete(reg_registry* reg, reg_entry* entry, reg_error* errPtr) {
+int reg_entry_delete(reg_entry* entry, reg_error* errPtr) {
+    reg_registry* reg = entry->reg;
+    int result = 0;
     sqlite3_stmt* stmt;
     char* query = "DELETE FROM registry.ports WHERE id=?";
-    if (!reg_test_writable(reg, errPtr)) {
-        return 0;
-    }
     if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
-            && (sqlite3_bind_int64(stmt, 1, entry->id) == SQLITE_OK)
-            && (sqlite3_step(stmt) == SQLITE_DONE)) {
-        if (sqlite3_changes(reg->db) > 0) {
-            sqlite3_finalize(stmt);
-            query = "DELETE FROM entries WHERE id=?";
-            if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
-                    && (sqlite3_bind_int64(stmt, 1, entry->id) == SQLITE_OK)
-                    && (sqlite3_step(stmt) == SQLITE_DONE)) {
-                sqlite3_finalize(stmt);
-                return 1;
-            } else {
-                reg_sqlite_error(reg->db, errPtr, query);
+            && (sqlite3_bind_int64(stmt, 1, entry->id) == SQLITE_OK)) {
+        int r;
+        do {
+            r = sqlite3_step(stmt);
+            switch (r) {
+                case SQLITE_DONE:
+                    if (sqlite3_changes(reg->db) > 0) {
+                        result = 1;
+                    } else {
+                        errPtr->code = REG_INVALID;
+                        errPtr->description = "an invalid entry was passed";
+                        errPtr->free = NULL;
+                    }
+                    break;
+                case SQLITE_BUSY:
+                    break;
+                case SQLITE_ERROR:
+                    reg_sqlite_error(reg->db, errPtr, query);
+                    break;
             }
-        } else {
-            errPtr->code = "registry::invalid";
-            errPtr->description = "an invalid entry was passed";
-            errPtr->free = NULL;
-        }
+        } while (r == SQLITE_BUSY);
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
     }
     sqlite3_finalize(stmt);
-    return 0;
+    return result;
 }
 
-/*
- * Frees the given entry - not good to expose?
+/**
+ * Frees an entry. Normally this is unnecessary, as open entries will be
+ * automatically freed when the registry is detached. Calling this method
+ * externally should only be necessary following `reg_entry_delete`.
+ *
+ * @param [in] entry the entry to free
  */
-static void reg_entry_free(reg_registry* reg UNUSED, reg_entry* entry) {
-    sqlite3_stmt* stmt;
+void reg_entry_free(reg_entry* entry) {
+    Tcl_HashEntry* hash = Tcl_FindHashEntry(&entry->reg->open_entries,
+                            (const char*)&entry->id);
+    Tcl_DeleteHashEntry(hash);
     if (entry->proc != NULL) {
         free(entry->proc);
     }
     free(entry);
-    sqlite3_prepare(entry->db, "DELETE FROM entries WHERE address=?", -1, &stmt,
-            NULL);
-    sqlite3_bind_blob(stmt, 1, &entry, sizeof(reg_entry*), SQLITE_TRANSIENT);
-    sqlite3_step(stmt);
 }
 
-static int reg_all_objects(sqlite3* db, char* query, int query_len,
+/**
+ * Convenience method for returning all objects of a given type from the
+ * registry.
+ *
+ * @param [in] reg       registry to select objects from
+ * @param [in] query     the select query to execute
+ * @param [in] query_len length of the query (or -1 for automatic)
+ * @param [out] objects  the objects selected
+ * @param [in] fn        a function to convert sqlite3_stmts to the desired type
+ * @param [in] del       a function to delete the desired type of object
+ * @param [out] errPtr   on error, a description of the error that occurred
+ * @return               the number of objects if success; negative if failure
+ */
+static int reg_all_objects(reg_registry* reg, char* query, int query_len,
         void*** objects, cast_function* fn, free_function* del,
         reg_error* errPtr) {
-    int r;
-    reg_entry* entry;
     void** results = malloc(10*sizeof(void*));
     int result_count = 0;
     int result_space = 10;
     sqlite3_stmt* stmt;
-    if (sqlite3_prepare(db, query, query_len, &stmt, NULL) == SQLITE_OK) {
+    if (sqlite3_prepare(reg->db, query, query_len, &stmt, NULL) == SQLITE_OK) {
+        int r;
+        reg_entry* entry;
         do {
-            int i;
             r = sqlite3_step(stmt);
             switch (r) {
                 case SQLITE_ROW:
-                    if (fn(db, (void**)&entry, stmt, errPtr)) {
+                    if (fn(reg, (void**)&entry, stmt, errPtr)) {
                         reg_listcat(&results, &result_count, &result_space,
                                 entry);
-                        continue;
                     } else {
-                        int i;
-                        sqlite3_finalize(stmt);
-                        for (i=0; i<result_count; i++) {
-                            del(NULL, results[i]);
-                        }
-                        free(results);
-                        return -1;
+                        r = SQLITE_ERROR;
                     }
+                    break;
                 case SQLITE_DONE:
+                case SQLITE_BUSY:
                     break;
                 default:
-                    for (i=0; i<result_count; i++) {
-                        del(NULL, results[i]);
-                    }
-                    free(results);
-                    sqlite3_finalize(stmt);
-                    reg_sqlite_error(db, errPtr, query);
-                    return -1;
+                    reg_sqlite_error(reg->db, errPtr, query);
+                    break;
             }
-        } while (r != SQLITE_DONE);
-        *objects = results;
-        return result_count;
+        } while (r == SQLITE_ROW || r == SQLITE_BUSY);
+        sqlite3_finalize(stmt);
+        if (r == SQLITE_DONE) {
+            *objects = results;
+            return result_count;
+        } else {
+            int i;
+            for (i=0; i<result_count; i++) {
+                del(NULL, results[i]);
+            }
+        }
     } else {
-        reg_sqlite_error(db, errPtr, query);
-        free(results);
-        return -1;
+        sqlite3_finalize(stmt);
+        reg_sqlite_error(reg->db, errPtr, query);
     }
+    free(results);
+    return -1;
 }
 
-/*
+/**
+ * Type-safe version of `reg_all_objects` for `reg_entry`.
+ *
+ * @param [in] reg       registry to select entries from
+ * @param [in] query     the select query to execute
+ * @param [in] query_len length of the query (or -1 for automatic)
+ * @param [out] objects  the entries selected
+ * @param [out] errPtr   on error, a description of the error that occurred
+ * @return               the number of entries if success; negative if failure
+ */
+static int reg_all_entries(reg_registry* reg, char* query, int query_len,
+        reg_entry*** objects, reg_error* errPtr) {
+    return reg_all_objects(reg, query, query_len, (void***)objects,
+            reg_stmt_to_entry, NULL, errPtr);
+}
+
+/**
  * Searches the registry for ports for which each key's value is equal to the
- * given value. To find all ports, pass 0 key-value pairs.
+ * given value. To find all ports, pass a key_count of 0.
  *
- * Vulnerable to SQL-injection attacks in the `keys` field. Pass it valid keys,
- * please.
+ * Bad keys should cause sqlite3 errors but not permit SQL injection attacks.
+ * Pass it good keys anyway.
+ *
+ * @param [in] reg       registry to search in
+ * @param [in] keys      a list of keys to search by
+ * @param [in] vals      a list of values to search by, matching keys
+ * @param [in] key_count the number of key/value pairs passed
+ * @param [in] strategy  strategy to use (one of the `reg_strategy_*` constants)
+ * @param [out] entries  a list of matching entries
+ * @param [out] errPtr   on error, a description of the error that occurred
+ * @return               the number of entries if success; false if failure
  */
 int reg_entry_search(reg_registry* reg, char** keys, char** vals, int key_count,
         int strategy, reg_entry*** entries, reg_error* errPtr) {
     int i;
     char* kwd = " WHERE ";
     char* query;
-    int query_len = 96;
-    int query_space = 96;
+    int query_len = 44;
+    int query_space = 44;
     int result;
     /* get the strategy */
     char* op = reg_strategy_op(strategy, errPtr);
@@ -404,23 +441,16 @@
         return -1;
     }
     /* build the query */
-    query = strdup("SELECT registry.ports.id, entries.address "
-            "FROM registry.ports LEFT OUTER JOIN entries USING (id)");
-    for (i=0; i<key_count; i+=1) {
-        char* cond = sqlite3_mprintf("%s%s%s'%q'", kwd, keys[i], op, vals[i]);
+    query = strdup("SELECT registry.ports.id FROM registry.ports");
+    for (i=0; i<key_count; i++) {
+        char* cond = sqlite3_mprintf(op, keys[i], vals[i]);
+        reg_strcat(&query, &query_len, &query_space, kwd);
         reg_strcat(&query, &query_len, &query_space, cond);
         sqlite3_free(cond);
         kwd = " AND ";
     }
     /* do the query */
-    result = reg_all_objects(reg->db, query, query_len, (void***)entries,
-            reg_stmt_to_entry, (free_function*)reg_entry_free, errPtr);
-    if (result > 0) {
-        if (!reg_save_addresses(reg->db, *entries, result, errPtr)) {
-            free(entries);
-            return -1;
-        }
-    }
+    result = reg_all_entries(reg, query, query_len, entries, errPtr);
     free(query);
     return result;
 }
@@ -436,15 +466,14 @@
  * @param [in] version  specific version to find (NULL for any)
  * @param [out] entries list of ports meeting the criteria
  * @param [out] errPtr  description of error encountered, if any
- * @return              the number of such ports found
+ * @return              the number of entries if success; false if failure
  */
 int reg_entry_imaged(reg_registry* reg, char* name, char* version, 
         reg_entry*** entries, reg_error* errPtr) {
     char* format;
     char* query;
     int result;
-    char* select = "SELECT registry.ports.id, entries.address FROM "
-        "registry.ports LEFT OUTER JOIN entries USING (id)";
+    char* select = "SELECT registry.ports.id FROM registry.ports";
     if (name == NULL) {
         format = "%s WHERE (state='imaged' OR state='installed')";
     } else if (version == NULL) {
@@ -454,20 +483,13 @@
             "AND version='%q'";
     }
     query = sqlite3_mprintf(format, select, name, version);
-    result = reg_all_objects(reg->db, query, -1, (void***)entries,
-            reg_stmt_to_entry, (free_function*)reg_entry_free, errPtr);
-    if (result > 0) {
-        if (!reg_save_addresses(reg->db, *entries, result, errPtr)) {
-            free(entries);
-            return -1;
-        }
-    }
+    result = reg_all_entries(reg, query, -1, entries, errPtr);
     sqlite3_free(query);
     return result;
 }
 
 /**
- * Finds ports which are active in the filesystem. These ports are able to fill
+ * Finds ports which are active in the filesystem. These ports are able to meet
  * dependencies, and properly own the files they map.
  * @todo add more arguments (epoch, revision, variants), maybe
  *
@@ -475,166 +497,238 @@
  * @param [in] name     specific port to find (NULL for any)
  * @param [out] entries list of ports meeting the criteria
  * @param [out] errPtr  description of error encountered, if any
- * @return              the number of such ports found
+ * @return              the number of entries if success; false if failure
  */
 int reg_entry_installed(reg_registry* reg, char* name, reg_entry*** entries,
         reg_error* errPtr) {
     char* format;
     char* query;
     int result;
-    char* select = "SELECT registry.ports.id, entries.address FROM "
-        "registry.ports LEFT OUTER JOIN entries USING (id)";
+    char* select = "SELECT registry.ports.id FROM registry.ports";
     if (name == NULL) {
         format = "%s WHERE state='installed'";
     } else {
         format = "%s WHERE state='installed' AND name='%q'";
     }
     query = sqlite3_mprintf(format, select, name);
-    result = reg_all_objects(reg->db, query, -1, (void***)entries,
-            reg_stmt_to_entry, (free_function*)reg_entry_free, errPtr);
-    if (result > 0) {
-        if (!reg_save_addresses(reg->db, *entries, result, errPtr)) {
-            free(entries);
-            return -1;
-        }
-    }
+    result = reg_all_entries(reg, query, -1, entries, errPtr);
     sqlite3_free(query);
     return result;
 }
 
+/**
+ * Finds the owner of a given file. Only ports active in the filesystem will be
+ * returned.
+ *
+ * @param [in] reg     registry to search in
+ * @param [in] path    path of the file to check ownership of
+ * @param [out] entry  the owner, or NULL if no active port owns the file
+ * @param [out] errPtr on error, a description of the error that occurred
+ * @return             true if success; false if failure
+ */
 int reg_entry_owner(reg_registry* reg, char* path, reg_entry** entry,
         reg_error* errPtr) {
+    int result = 0;
     sqlite3_stmt* stmt;
-    reg_entry* result;
-    char* query = "SELECT registry.files.id, entries.address "
-        "FROM registry.files INNER JOIN registry.ports USING(id)"
-        " LEFT OUTER JOIN entries USING(id) "
-        "WHERE path=? AND registry.ports.state = 'installed'";
+    char* query = "SELECT registry.files.id FROM registry.files INNER JOIN "
+        "registry.ports USING(id) WHERE  registry.ports.state = 'installed' "
+        "AND registry.files.path=?";
     if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
             && (sqlite3_bind_text(stmt, 1, path, -1, SQLITE_STATIC)
                 == SQLITE_OK)) {
-        int r = sqlite3_step(stmt);
-        switch (r) {
-            case SQLITE_ROW:
-                if (reg_stmt_to_entry(reg->db, (void**)&result, stmt, errPtr)) {
-                    sqlite3_finalize(stmt);
-                    if (reg_save_addresses(reg->db, &result, 1, errPtr)) {
-                        *entry = result;
-                        return 1;
-                    }
-                }
-            case SQLITE_DONE:
-                sqlite3_finalize(stmt);
-                *entry = NULL;
-                return 1;
-            default:
-                /* barf */
-                sqlite3_finalize(stmt);
-                return 0;
-        }
+        int r;
+        do {
+            r = sqlite3_step(stmt);
+            switch (r) {
+                case SQLITE_ROW:
+                    result = reg_stmt_to_entry(reg, (void**)entry, stmt,
+                            errPtr);
+                    break;
+                case SQLITE_DONE:
+                    *entry = NULL;
+                    result = 1;
+                    break;
+                case SQLITE_BUSY:
+                    break;
+                default:
+                    reg_sqlite_error(reg->db, errPtr, query);
+                    break;
+            }
+        } while (r == SQLITE_BUSY);
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
-        sqlite3_finalize(stmt);
-        return 0;
     }
+    sqlite3_finalize(stmt);
+    return result;
 }
 
-int reg_entry_propget(reg_registry* reg, reg_entry* entry, char* key,
-        char** value, reg_error* errPtr) {
+/**
+ * Shortcut to retrieve a file's owner's id directly. This should be a bit
+ * faster than `reg_entry_owner` because it doesn't have to check the hash table
+ * of open entries. It might still be slower than necessary because it's doing
+ * a join of two tables, but the only way to improve that would be to cache a
+ * file's active state in registry.files itself, which I'd rather not do unless
+ * absolutely necessary.
+ *
+ * @param [in] reg  registry to find file in
+ * @param [in] path path of file to get owner of
+ * @return          id of owner, or 0 for none
+ */
+sqlite_int64 reg_entry_owner_id(reg_registry* reg, char* path) {
     sqlite3_stmt* stmt;
+    sqlite_int64 result = 0;
+    char* query = "SELECT registry.files.id FROM registry.files INNER JOIN "
+        "registry.ports USING(id) WHERE  registry.ports.state = 'installed' "
+        "AND registry.files.path=?";
+    if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
+            && (sqlite3_bind_text(stmt, 1, path, -1, SQLITE_STATIC)
+                == SQLITE_OK)) {
+        int r;
+        do {
+            r = sqlite3_step(stmt);
+            if (r == SQLITE_ROW) {
+                result = sqlite3_column_int64(stmt, 0);
+            }
+        } while (r == SQLITE_BUSY);
+    }
+    sqlite3_finalize(stmt);
+    return result;
+}
+
+/**
+ * Gets a named property of an entry. That property can be set using
+ * `reg_entry_propset`. The property named must be one that exists in the table
+ * and must not be one with internal meaning such as `id` or `state`.
+ *
+ * @param [in] entry   entry to get property from
+ * @param [in] key     property to get
+ * @param [out] value  the value of the property
+ * @param [out] errPtr on error, a description of the error that occurred
+ * @return             true if success; false if failure
+ */
+int reg_entry_propget(reg_entry* entry, char* key, char** value,
+        reg_error* errPtr) {
+    reg_registry* reg = entry->reg;
+    int result = 0;
+    sqlite3_stmt* stmt;
     char* query;
     query = sqlite3_mprintf("SELECT %q FROM registry.ports WHERE id=%lld", key,
             entry->id);
     if (sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK) {
         int r = sqlite3_step(stmt);
-        switch (r) {
-            case SQLITE_ROW:
-                *value = strdup((const char*)sqlite3_column_text(stmt, 0));
-                sqlite3_finalize(stmt);
-                sqlite3_free(query);
-                return 1;
-            case SQLITE_DONE:
-                errPtr->code = "registry::invalid";
-                errPtr->description = "an invalid entry was passed";
-                errPtr->free = NULL;
-                sqlite3_finalize(stmt);
-                sqlite3_free(query);
-                return 0;
-            default:
-                reg_sqlite_error(reg->db, errPtr, query);
-                sqlite3_finalize(stmt);
-                sqlite3_free(query);
-                return 0;
-        }
+        do {
+            switch (r) {
+                case SQLITE_ROW:
+                    *value = strdup((const char*)sqlite3_column_text(stmt, 0));
+                    result = 1;
+                    break;
+                case SQLITE_DONE:
+                    errPtr->code = REG_INVALID;
+                    errPtr->description = "an invalid entry was passed";
+                    errPtr->free = NULL;
+                    break;
+                case SQLITE_BUSY:
+                    continue;
+                default:
+                    reg_sqlite_error(reg->db, errPtr, query);
+                    break;
+            }
+        } while (r == SQLITE_BUSY);
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
-        sqlite3_free(query);
-        return 0;
     }
+    sqlite3_finalize(stmt);
+    sqlite3_free(query);
+    return result;
 }
 
-int reg_entry_propset(reg_registry* reg, reg_entry* entry, char* key,
-        char* value, reg_error* errPtr) {
+/**
+ * Sets a named property of an entry. That property can be later retrieved using
+ * `reg_entry_propget`. The property named must be one that exists in the table
+ * and must not be one with internal meaning such as `id` or `state`. If `name`,
+ * `epoch`, `version`, `revision`, or `variants` is set, it could trigger a
+ * conflict if another port with the same combination of values for those
+ * columns exists.
+ *
+ * @param [in] entry   entry to set property for
+ * @param [in] key     property to set
+ * @param [in] value   the desired value of the property
+ * @param [out] errPtr on error, a description of the error that occurred
+ * @return             true if success; false if failure
+ */
+int reg_entry_propset(reg_entry* entry, char* key, char* value,
+        reg_error* errPtr) {
+    reg_registry* reg = entry->reg;
+    int result = 0;
     sqlite3_stmt* stmt;
     char* query;
-    if (!reg_test_writable(reg, errPtr)) {
-        return -1;
-    }
     query = sqlite3_mprintf("UPDATE registry.ports SET %q = '%q' WHERE id=%lld",
             key, value, entry->id);
     if (sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK) {
-        int r = sqlite3_step(stmt);
-        switch (r) {
-            case SQLITE_DONE:
-                sqlite3_free(query);
-                sqlite3_finalize(stmt);
-                return 1;
-            default:
-                switch (sqlite3_reset(stmt)) {
-                    case SQLITE_CONSTRAINT:
-                        errPtr->code = "registry::constraint";
+        int r;
+        do {
+            r = sqlite3_step(stmt);
+            switch (r) {
+                case SQLITE_DONE:
+                    result = 1;
+                    break;
+                case SQLITE_BUSY:
+                    break;
+                default:
+                    if (sqlite3_reset(stmt) == SQLITE_CONSTRAINT) {
+                        errPtr->code = REG_CONSTRAINT;
                         errPtr->description = "a constraint was disobeyed";
                         errPtr->free = NULL;
-                        sqlite3_finalize(stmt);
-                        sqlite3_free(query);
-                        return 0;
-                    default:
+                    } else {
                         reg_sqlite_error(reg->db, errPtr, query);
-                        sqlite3_finalize(stmt);
-                        sqlite3_free(query);
-                        return 0;
-                }
-        }
+                    }
+                    break;
+            }
+        } while (r == SQLITE_BUSY);
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
-        sqlite3_free(query);
-        return 0;
     }
+    sqlite3_finalize(stmt);
+    sqlite3_free(query);
+    return result;
 }
 
-int reg_entry_map(reg_registry* reg, reg_entry* entry, char** files,
-        int file_count, reg_error* errPtr) {
+/**
+ * Maps files to the given port in the filemap. The list of files must not
+ * contain files that are already mapped to the given port.
+ *
+ * @param [in] entry      the entry to map the files to
+ * @param [in] files      a list of files to map
+ * @param [in] file_count the number of files
+ * @param [out] errPtr    on error, a description of the error that occurred
+ * @return                true if success; false if failure
+ */
+int reg_entry_map(reg_entry* entry, char** files, int file_count,
+        reg_error* errPtr) {
+    reg_registry* reg = entry->reg;
     sqlite3_stmt* stmt;
     char* insert = "INSERT INTO registry.files (id, path) VALUES (?, ?)";
-    if (!reg_test_writable(reg, errPtr)) {
-        return -1;
-    }
     if ((sqlite3_prepare(reg->db, insert, -1, &stmt, NULL) == SQLITE_OK)
             && (sqlite3_bind_int64(stmt, 1, entry->id) == SQLITE_OK)) {
         int i;
         for (i=0; i<file_count; i++) {
             if (sqlite3_bind_text(stmt, 2, files[i], -1, SQLITE_STATIC)
                     == SQLITE_OK) {
-                int r = sqlite3_step(stmt);
-                switch (r) {
-                    case SQLITE_DONE:
-                        sqlite3_reset(stmt);
-                        continue;
-                    default:
-                        reg_sqlite_error(reg->db, errPtr, insert);
-                        sqlite3_finalize(stmt);
-                        return i;
-                }
+                int r;
+                do {
+                    r = sqlite3_step(stmt);
+                    switch (r) {
+                        case SQLITE_DONE:
+                            sqlite3_reset(stmt);
+                            break;
+                        case SQLITE_BUSY:
+                            break;
+                        default:
+                            reg_sqlite_error(reg->db, errPtr, insert);
+                            sqlite3_finalize(stmt);
+                            return i;
+                    }
+                } while (r == SQLITE_BUSY);
             } else {
                 reg_sqlite_error(reg->db, errPtr, insert);
                 sqlite3_finalize(stmt);
@@ -650,38 +744,50 @@
     }
 }
 
-int reg_entry_unmap(reg_registry* reg, reg_entry* entry, char** files,
-        int file_count, reg_error* errPtr) {
+/**
+ * Unaps files from the given port in the filemap. The files must be owned by
+ * the given entry.
+ *
+ * @param [in] entry      the entry to unmap the files from
+ * @param [in] files      a list of files to unmap
+ * @param [in] file_count the number of files
+ * @param [out] errPtr    on error, a description of the error that occurred
+ * @return                true if success; false if failure
+ */
+int reg_entry_unmap(reg_entry* entry, char** files, int file_count,
+        reg_error* errPtr) {
+    reg_registry* reg = entry->reg;
     sqlite3_stmt* stmt;
     char* query = "DELETE FROM registry.files WHERE id=? AND path=?";
-    if (!reg_test_writable(reg, errPtr)) {
-        return -1;
-    }
     if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
             && (sqlite3_bind_int64(stmt, 1, entry->id) == SQLITE_OK)) {
         int i;
         for (i=0; i<file_count; i++) {
             if (sqlite3_bind_text(stmt, 2, files[i], -1, SQLITE_STATIC)
                     == SQLITE_OK) {
-                int r = sqlite3_step(stmt);
-                switch (r) {
-                    case SQLITE_DONE:
-                        if (sqlite3_changes(reg->db) == 0) {
-                            errPtr->code = "registry::invalid";
-                            errPtr->description = "this entry does not own the "
-                                "given file";
-                            errPtr->free = NULL;
+                int r;
+                do {
+                    r = sqlite3_step(stmt);
+                    switch (r) {
+                        case SQLITE_DONE:
+                            if (sqlite3_changes(reg->db) == 0) {
+                                errPtr->code = REG_INVALID;
+                                errPtr->description = "this entry does not own "
+                                    "the given file";
+                                errPtr->free = NULL;
+                                sqlite3_finalize(stmt);
+                                return i;
+                            }
+                            sqlite3_reset(stmt);
+                            break;
+                        case SQLITE_BUSY:
+                            break;
+                        default:
+                            reg_sqlite_error(reg->db, errPtr, query);
                             sqlite3_finalize(stmt);
                             return i;
-                        } else {
-                            sqlite3_reset(stmt);
-                            continue;
-                        }
-                    default:
-                        reg_sqlite_error(reg->db, errPtr, query);
-                        sqlite3_finalize(stmt);
-                        return i;
-                }
+                    }
+                } while (r == SQLITE_BUSY);
             } else {
                 reg_sqlite_error(reg->db, errPtr, query);
                 sqlite3_finalize(stmt);
@@ -697,8 +803,16 @@
     }
 }
 
-int reg_entry_files(reg_registry* reg, reg_entry* entry, char*** files,
-        reg_error* errPtr) {
+/**
+ * Gets a list of files owned by the given port.
+ *
+ * @param [in] entry   entry to get the list for
+ * @param [out] files  a list of files owned by the port
+ * @param [out] errPtr on error, a description of the error that occurred
+ * @return             the number of files if success; negative if failure
+ */
+int reg_entry_files(reg_entry* entry, char*** files, reg_error* errPtr) {
+    reg_registry* reg = entry->reg;
     sqlite3_stmt* stmt;
     char* query = "SELECT path FROM registry.files WHERE id=? ORDER BY path";
     if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
@@ -709,29 +823,33 @@
         int r;
         do {
             char* element;
-            int i;
             r = sqlite3_step(stmt);
             switch (r) {
                 case SQLITE_ROW:
                     element = strdup((const char*)sqlite3_column_text(stmt, 0));
-                    reg_listcat((void*)&result, &result_count, &result_space,
+                    reg_listcat((void***)&result, &result_count, &result_space,
                             element);
                     break;
                 case SQLITE_DONE:
+                case SQLITE_BUSY:
                     break;
                 default:
-                    for (i=0; i<result_count; i++) {
-                        free(result[i]);
-                    }
-                    free(result);
                     reg_sqlite_error(reg->db, errPtr, query);
-                    sqlite3_finalize(stmt);
-                    return -1;
+                    break;
             }
-        } while (r != SQLITE_DONE);
+        } while (r == SQLITE_ROW || r == SQLITE_BUSY);
         sqlite3_finalize(stmt);
-        *files = result;
-        return result_count;
+        if (r == SQLITE_DONE) {
+            *files = result;
+            return result_count;
+        } else {
+            int i;
+            for (i=0; i<result_count; i++) {
+                free(result[i]);
+            }
+            free(result);
+            return -1;
+        }
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
         sqlite3_finalize(stmt);
@@ -739,31 +857,25 @@
     }
 }
 
-int reg_all_entries(reg_registry* reg, reg_entry*** entries, reg_error* errPtr){
+/**
+ * Fetches a list of all open entries.
+ *
+ * @param [in] reg      registry to fetch entries from
+ * @param [out] entries a list of open entries
+ * @return              the number of open entries
+ */
+int reg_all_open_entries(reg_registry* reg, reg_entry*** entries) {
     reg_entry* entry;
-    void** results = malloc(10*sizeof(void*));
-    int result_count = 0;
-    int result_space = 10;
-    sqlite3_stmt* stmt;
-    char* query = "SELECT address FROM entries";
-    if (sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK) {
-        int r;
-        do {
-            r = sqlite3_step(stmt);
-            switch (r) {
-                case SQLITE_ROW:
-                    entry = *(reg_entry**)sqlite3_column_blob(stmt, 0);
-                    reg_listcat(&results, &result_count, &result_space, entry);
-                    break;
-                case SQLITE_DONE:
-                    break;
-                default:
-                    reg_sqlite_error(reg->db, errPtr, query);
-                    free(results);
-                    return -1;
-            }
-        } while (r != SQLITE_DONE);
+    int entry_count = 0;
+    int entry_space = 10;
+    Tcl_HashEntry* hash;
+    Tcl_HashSearch search;
+    *entries = malloc(10*sizeof(void*));
+    for (hash = Tcl_FirstHashEntry(&reg->open_entries, &search); hash != NULL;
+            hash = Tcl_NextHashEntry(&search)) {
+        entry = Tcl_GetHashValue(hash);
+        reg_listcat((void***)entries, &entry_count, &entry_space, entry);
     }
-    *entries = (reg_entry**)results;
-    return result_count;
+    return entry_count;
 }
+

Modified: trunk/base/src/cregistry/entry.h
===================================================================
--- trunk/base/src/cregistry/entry.h	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/cregistry/entry.h	2007-08-06 20:13:30 UTC (rev 27528)
@@ -36,15 +36,14 @@
 #include <cregistry/registry.h>
 
 typedef enum {
-    reg_strategy_equal = 1,
+    reg_strategy_exact = 1,
     reg_strategy_glob = 2,
     reg_strategy_regexp = 3
 } reg_strategy;
 
 typedef struct {
     sqlite_int64 id; /* rowid in database */
-    sqlite3* db; /* database */
-    int saved; /* have we recorded &entry in database? */
+    reg_registry* reg; /* associated registry */
     char* proc; /* name of Tcl proc, if using Tcl */
 } reg_entry;
 
@@ -54,8 +53,10 @@
 reg_entry* reg_entry_open(reg_registry* reg, char* name, char* version,
         char* revision, char* variants, char* epoch, reg_error* errPtr);
 
-int reg_entry_delete(reg_registry* reg, reg_entry* entry, reg_error* errPtr);
+int reg_entry_delete(reg_entry* entry, reg_error* errPtr);
 
+void reg_entry_free(reg_entry* entry);
+
 int reg_entry_search(reg_registry* reg, char** keys, char** vals, int key_count,
         int strategy, reg_entry*** entries, reg_error* errPtr);
 
@@ -67,19 +68,18 @@
 int reg_entry_owner(reg_registry* reg, char* path, reg_entry** entry,
         reg_error* errPtr);
 
-int reg_entry_propget(reg_registry* reg, reg_entry* entry, char* key,
-        char** value, reg_error* errPtr);
-int reg_entry_propset(reg_registry* reg, reg_entry* entry, char* key,
-        char* value, reg_error* errPtr);
-
-int reg_entry_map(reg_registry* reg, reg_entry* entry, char** files, int file_count,
+int reg_entry_propget(reg_entry* entry, char* key, char** value,
         reg_error* errPtr);
-int reg_entry_unmap(reg_registry* reg, reg_entry* entry, char** files, int file_count,
+int reg_entry_propset(reg_entry* entry, char* key, char* value,
         reg_error* errPtr);
 
-int reg_entry_files(reg_registry* reg, reg_entry* entry, char*** files,
+int reg_entry_map(reg_entry* entry, char** files, int file_count,
         reg_error* errPtr);
+int reg_entry_unmap(reg_entry* entry, char** files, int file_count,
+        reg_error* errPtr);
 
-int reg_all_entries(reg_registry*, reg_entry*** entries, reg_error* errPtr);
+int reg_entry_files(reg_entry* entry, char*** files, reg_error* errPtr);
 
+int reg_all_open_entries(reg_registry* reg, reg_entry*** entries);
+
 #endif /* _CENTRY_H */

Modified: trunk/base/src/cregistry/registry.c
===================================================================
--- trunk/base/src/cregistry/registry.c	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/cregistry/registry.c	2007-08-06 20:13:30 UTC (rev 27528)
@@ -33,6 +33,7 @@
 #include <stdio.h>
 #include <unistd.h>
 #include <stdlib.h>
+#include <libgen.h>
 #include <sqlite3.h>
 #include <sys/stat.h>
 #include <errno.h>
@@ -40,6 +41,17 @@
 #include <cregistry/entry.h>
 #include <cregistry/sql.h>
 
+/*
+ * TODO: create a better system for throwing errors. It'd be really awesome to
+ *       have e.g. `reg_throw_error(code, fmt, ...)` but I don't feel like
+ *       messing with varargs right now.
+ *
+ * TODO: maybe all the errPtrs could be made a property of `reg_registry`
+ *       instead, and be destroyed with it? That would make memory-management
+ *       much easier for errors, and there's no need to have more than one error
+ *       alive at any given time.
+ */
+
 /**
  * Destroys a `reg_error` object. This should be called on any reg_error when a
  * registry function returns a failure condition; depending on the function,
@@ -55,15 +67,15 @@
 
 /**
  * Sets `errPtr` according to the last error in `db`. Convenience function for
- * internal use only. Sets the error code to "registry::sqlite-error" and sets
- * an appropriate error description (including a query if non-NULL).
+ * internal use only. Sets the error code to REG_SQLITE_ERROR and sets an
+ * appropriate error description (including a query if non-NULL).
  *
  * @param [in] db      sqlite3 database connection which had the error
  * @param [out] errPtr an error to write to
  * @param [in] query   the query that this error occurred during
  */
 void reg_sqlite_error(sqlite3* db, reg_error* errPtr, char* query) {
-    errPtr->code = "registry::sqlite-error";
+    errPtr->code = REG_SQLITE_ERROR;
     errPtr->free = (reg_error_destructor*)sqlite3_free;
     if (query == NULL) {
         errPtr->description = sqlite3_mprintf("sqlite error: %s",
@@ -113,7 +125,7 @@
         free(reg);
         return 1;
     } else {
-        errPtr->code = "registry::sqlite-error";
+        errPtr->code = REG_SQLITE_ERROR;
         errPtr->description = sqlite3_mprintf("registry db not closed "
                 "correctly (%s)\n", sqlite3_errmsg(reg->db));
         errPtr->free = (reg_error_destructor*)sqlite3_free;
@@ -133,43 +145,75 @@
  */
 int reg_attach(reg_registry* reg, const char* path, reg_error* errPtr) {
     struct stat sb;
-    int needsInit = 0; /* registry doesn't yet exist */
-    int canWrite = 1; /* can write to this location */
+    int initialized = 1; /* registry already exists */
+    int can_write = 1; /* can write to this location */
+    int result = 0;
     if (reg->status & reg_attached) {
-        errPtr->code = "registry::misuse";
+        errPtr->code = REG_MISUSE;
         errPtr->description = "a database is already attached to this registry";
         errPtr->free = NULL;
         return 0;
     }
     if (stat(path, &sb) != 0) {
+        initialized = 0;
         if (errno == ENOENT) {
-            needsInit = 1;
+            if (stat(dirname(path), &sb) != 0) {
+                can_write = 0;
+            }
         } else {
-            canWrite = 0;
+            can_write = 0;
         }
     }
-    if (!needsInit || canWrite) {
+    /* can_write is still true if one of the stat calls succeeded */
+    if (can_write) {
+        if (sb.st_uid == getuid()) {
+            if (!(sb.st_mode & S_IWUSR)) {
+                can_write = 0;
+            }
+        } else if (sb.st_gid == getgid()) {
+            if (!(sb.st_mode & S_IWGRP)) {
+                can_write = 0;
+            }
+        } else {
+            if (!(sb.st_mode & S_IWOTH)) {
+                can_write = 0;
+            }
+        }
+    }
+    if (initialized || can_write) {
         sqlite3_stmt* stmt;
         char* query = sqlite3_mprintf("ATTACH DATABASE '%q' AS registry", path);
-        if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
-                && (sqlite3_step(stmt) == SQLITE_DONE)) {
-            sqlite3_finalize(stmt);
-            sqlite3_free(query);
-            if (!needsInit || (create_tables(reg->db, errPtr))) {
-                reg->status |= reg_attached;
-                return 1;
-            }
+        if (sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK) {
+            int r;
+            do {
+                r = sqlite3_step(stmt);
+                switch (r) {
+                    case SQLITE_DONE:
+                        if (initialized || (create_tables(reg->db, errPtr))) {
+                            Tcl_InitHashTable(&reg->open_entries,
+                                    sizeof(sqlite_int64)/sizeof(int));
+                            reg->status |= reg_attached;
+                            result = 1;
+                        }
+                        break;
+                    case SQLITE_BUSY:
+                        break;
+                    default:
+                        reg_sqlite_error(reg->db, errPtr, query);
+                }
+            } while (r == SQLITE_BUSY);
         } else {
             reg_sqlite_error(reg->db, errPtr, query);
-            sqlite3_finalize(stmt);
-            sqlite3_free(query);
         }
+        sqlite3_finalize(stmt);
+        sqlite3_free(query);
     } else {
-        errPtr->code = "registry::cannot-init";
-        errPtr->description = sqlite3_mprintf("port registry doesn't exist at \"%q\" and couldn't write to this location", path);
+        errPtr->code = REG_CANNOT_INIT;
+        errPtr->description = sqlite3_mprintf("port registry doesn't exist at "
+                "\"%q\" and couldn't write to this location", path);
         errPtr->free = (reg_error_destructor*)sqlite3_free;
     }
-    return 0;
+    return result;
 }
 
 /**
@@ -183,54 +227,47 @@
  */
 int reg_detach(reg_registry* reg, reg_error* errPtr) {
     sqlite3_stmt* stmt;
+    int result = 0;
     char* query = "DETACH DATABASE registry";
     if (!(reg->status & reg_attached)) {
-        errPtr->code = "registry::misuse";
+        errPtr->code = REG_MISUSE;
         errPtr->description = "no database is attached to this registry";
         errPtr->free = NULL;
         return 0;
     }
-    if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK)
-            && (sqlite3_step(stmt) == SQLITE_DONE)) {
-        sqlite3_finalize(stmt);
-        query = "SELECT address FROM entries";
-        if (sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK) {
-            int r;
-            reg_entry* entry;
-            do {
-                r = sqlite3_step(stmt);
-                switch (r) {
-                    case SQLITE_ROW:
-                        entry = *(reg_entry**)sqlite3_column_blob(stmt, 0);
-                        if (entry->proc != NULL) {
+    if (sqlite3_prepare(reg->db, query, -1, &stmt, NULL) == SQLITE_OK) {
+        int r;
+        reg_entry* entry;
+        Tcl_HashEntry* curr;
+        Tcl_HashSearch search;
+        do {
+            r = sqlite3_step(stmt);
+            switch (r) {
+                case SQLITE_DONE:
+                    for (curr = Tcl_FirstHashEntry(&reg->open_entries, &search);
+                            curr != NULL; curr = Tcl_NextHashEntry(&search)) {
+                        entry = Tcl_GetHashValue(curr);
+                        if (entry->proc) {
                             free(entry->proc);
                         }
                         free(entry);
-                        /* reg_entry_free(reg->db, entry); */
-                        break;
-                    case SQLITE_DONE:
-                        break;
-                    default:
-                        reg_sqlite_error(reg->db, errPtr, query);
-                        return 0;
-                }
-            } while (r != SQLITE_DONE);
-        }
-        sqlite3_finalize(stmt);
-        query = "DELETE FROM entries";
-        if ((sqlite3_prepare(reg->db, query, -1, &stmt, NULL) != SQLITE_OK)
-                || (sqlite3_step(stmt) != SQLITE_DONE)) {
-            sqlite3_finalize(stmt);
-            return 0;
-        }
-        sqlite3_finalize(stmt);
-        reg->status &= ~reg_attached;
-        return 1;
+                    }
+                    Tcl_DeleteHashTable(&reg->open_entries);
+                    reg->status &= ~reg_attached;
+                    result = 1;
+                    break;
+                case SQLITE_BUSY:
+                    break;
+                default:
+                    reg_sqlite_error(reg->db, errPtr, query);
+                    break;
+            }
+        } while (r == SQLITE_BUSY);
     } else {
         reg_sqlite_error(reg->db, errPtr, query);
-        sqlite3_finalize(stmt);
-        return 0;
     }
+    sqlite3_finalize(stmt);
+    return result;
 }
 
 /**
@@ -238,21 +275,21 @@
  */
 static int reg_start(reg_registry* reg, const char* query, reg_error* errPtr) {
     if (reg->status & reg_transacting) {
-        errPtr->code = "registry::misuse";
+        errPtr->code = REG_MISUSE;
         errPtr->description = "couldn't start transaction because a "
             "transaction is already open";
         errPtr->free = NULL;
         return 0;
     } else {
-        int result;
+        int r;
         do {
-            result = sqlite3_exec(reg->db, query, NULL, NULL, NULL);
-            if (result == SQLITE_ERROR) {
-                reg_sqlite_error(reg->db, errPtr, NULL);
-                return 0;
+            r = sqlite3_exec(reg->db, query, NULL, NULL, NULL);
+            if (r == SQLITE_OK) {
+                return 1;
             }
-        } while (result != SQLITE_OK);
-        return 1;
+        } while (r == SQLITE_BUSY);
+        reg_sqlite_error(reg->db, errPtr, NULL);
+        return 0;
     }
 }
 
@@ -296,21 +333,21 @@
  */
 static int reg_end(reg_registry* reg, const char* query, reg_error* errPtr) {
     if (!(reg->status & reg_transacting)) {
-        errPtr->code = "registry::misuse";
+        errPtr->code = REG_MISUSE;
         errPtr->description = "couldn't end transaction because no transaction "
             "is open";
         errPtr->free = NULL;
         return 0;
     } else {
-        int result;
+        int r;
         do {
-            result = sqlite3_exec(reg->db, query, NULL, NULL, NULL);
-            if (result == SQLITE_ERROR) {
-                reg_sqlite_error(reg->db, errPtr, NULL);
-                return 0;
+            r = sqlite3_exec(reg->db, query, NULL, NULL, NULL);
+            if (r == SQLITE_OK) {
+                return 1;
             }
-        } while (result != SQLITE_OK);
-        return 1;
+        } while (r == SQLITE_BUSY);
+        reg_sqlite_error(reg->db, errPtr, NULL);
+        return 0;
     }
 }
 
@@ -347,23 +384,3 @@
         return 0;
     }
 }
-
-/**
- * Ensures the registry has a write transaction open. If it doesn't, returns
- * false and sets an appropriate error. Mainly intended for internal use, though
- * there's no reason it couldn't be used externally.
- *
- * @param [in] reg     registry to check writability of
- * @param [out] errPtr if not writable, an error describing that situation
- * @return             true if writable; false if not
- */
-int reg_test_writable(reg_registry* reg, reg_error* errPtr) {
-    if (reg->status & reg_can_write) {
-        return 1;
-    } else {
-        errPtr->code = "registry::misuse";
-        errPtr->description = "a write transaction has not been started";
-        errPtr->free = NULL;
-        return 0;
-    }
-}

Modified: trunk/base/src/cregistry/registry.h
===================================================================
--- trunk/base/src/cregistry/registry.h	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/cregistry/registry.h	2007-08-06 20:13:30 UTC (rev 27528)
@@ -33,7 +33,15 @@
 #endif
 
 #include <sqlite3.h>
+#include <tcl.h>
 
+#define REG_NOT_FOUND       "registry::not-found"
+#define REG_INVALID         "registry::invalid"
+#define REG_CONSTRAINT      "registry::constraint"
+#define REG_SQLITE_ERROR    "registry::sqlite-error"
+#define REG_MISUSE          "registry::misuse"
+#define REG_CANNOT_INIT     "registry::cannot-init"
+
 typedef void reg_error_destructor(const char* description);
 
 typedef struct {
@@ -59,6 +67,7 @@
 typedef struct {
     sqlite3* db;
     int status;
+    Tcl_HashTable open_entries;
 } reg_registry;
 
 int reg_open(reg_registry** regPtr, reg_error* errPtr);
@@ -72,6 +81,4 @@
 int reg_commit(reg_registry* reg, reg_error* errPtr);
 int reg_rollback(reg_registry* reg, reg_error* errPtr);
 
-int reg_test_writable(reg_registry* reg, reg_error* errPtr);
-
 #endif /* _CREG_H */

Modified: trunk/base/src/cregistry/sql.c
===================================================================
--- trunk/base/src/cregistry/sql.c	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/cregistry/sql.c	2007-08-06 20:13:30 UTC (rev 27528)
@@ -39,6 +39,20 @@
 #include <cregistry/registry.h>
 #include <cregistry/sql.h>
 
+/*
+ * TODO: maybe this could be made into something that could be separately loaded
+ *       by sqlite3? It's a bit hard to query the registry with the command-line
+ *       sqlite3 tool because of the missing VERSION collation. My understanding
+ *       is that you can make a dylib that can be loaded using an sql statement,
+ *       which is less than transparent, but certainly reasonable.
+ *
+ * TODO: break out rpm_vercomp into a separate file which can be shared by
+ *       pextlib and cregistry. The version here is slightly modified so as to
+ *       take explicit string lengths. Since these are available in Tcl it's an
+ *       easy change and might be a tiny bit faster; it's necessary for the
+ *       application here.
+ */
+
 /**
  * Executes a null-terminated list of queries. Pass it a list of queries, it'll
  * execute them. This is mainly intended for initialization, when you have a
@@ -310,9 +324,6 @@
         /* indexes list */
         "CREATE TEMPORARY TABLE indexes (file, name, attached)",
 
-        /* entry addresses */
-        "CREATE TEMPORARY TABLE entries (id, address)",
-
         "COMMIT",
         NULL
     };

Modified: trunk/base/src/registry2.0/entry.c
===================================================================
--- trunk/base/src/registry2.0/entry.c	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/registry2.0/entry.c	2007-08-06 20:13:30 UTC (rev 27528)
@@ -60,7 +60,7 @@
  *
  * @param [in] clientData address of a reg_entry to remove
  */
-static void delete_entry(ClientData clientData) {
+void delete_entry(ClientData clientData) {
     reg_entry* entry = (reg_entry*)clientData;
     free(entry->proc);
     entry->proc = NULL;
@@ -78,7 +78,7 @@
  */
 static int set_entry(Tcl_Interp* interp, char* name, reg_entry* entry,
         reg_error* errPtr) {
-    if (set_object(interp, name, entry, "entry", entry_obj_cmd, delete_entry,
+    if (set_object(interp, name, entry, "entry", entry_obj_cmd, NULL,
                 errPtr)) {
         int size = strlen(name) + 1;
         entry->proc = malloc(size*sizeof(char));
@@ -164,7 +164,8 @@
 /**
  * registry::entry delete entry
  *
- * Deletes an entry from the registry (then closes it).
+ * Deletes an entry from the registry (then closes it). If this is done within a
+ * transaction and the transaction is rolled back, the entry will remain valid.
  */
 static int entry_delete(Tcl_Interp* interp, int objc, Tcl_Obj* CONST objv[]) {
     reg_registry* reg = registry_for(interp, reg_attached);
@@ -174,15 +175,29 @@
     } if (reg == NULL) {
         return TCL_ERROR;
     } else {
-        reg_entry* entry;
         reg_error error;
-        if (obj_to_entry(interp, &entry, objv[2], &error)) {
-            if (reg_entry_delete(reg, entry, &error)) {
-                Tcl_DeleteCommand(interp, Tcl_GetString(objv[2]));
-                return TCL_OK;
-            }
+        reg_entry* entry = get_entry(interp, Tcl_GetString(objv[2]), &error);
+        entry_list** list_handle;
+        if (entry == NULL) {
+            return registry_failed(interp, &error);
         }
-        return registry_failed(interp, &error);
+        if (!reg_entry_delete(entry, &error)) {
+            return registry_failed(interp, &error);
+        }
+        /* if there's a transaction going on, record this entry in a list so we
+         * can roll it back if necessary
+         */
+        list_handle = Tcl_GetAssocData(interp, "registry::deleted", NULL);
+        if (list_handle) {
+            entry_list* list = *list_handle;
+            *list_handle = malloc(sizeof(entry_list*));
+            (*list_handle)->entry = entry;
+            (*list_handle)->next = list;
+        } else {
+            reg_entry_free(entry);
+        }
+        Tcl_DeleteCommand(interp, Tcl_GetString(objv[2]));
+        return TCL_OK;
     }
 }
 
@@ -242,19 +257,33 @@
     }
 }
 
+typedef struct {
+    char* name;
+    reg_strategy strategy;
+} strategy_type;
+
+static strategy_type strategies[] = {
+    { "-exact",  reg_strategy_exact },
+    { "-glob",   reg_strategy_glob },
+    { "-regexp", reg_strategy_regexp },
+    { "--",      reg_strategy_exact },
+    { NULL, 0 }
+};
+
 /*
  * registry::entry search ?key value ...?
  *
  * Searches the registry for ports for which each key's value is equal to the
  * given value. To find all ports, call `entry search` with no key-value pairs.
- *
- * TODO: allow selection of -exact, -glob, and -regexp matching.
+ * Can be given an option of -exact, -glob, or -regexp to specify the matching
+ * strategy; defaults to exact.
  */
 static int entry_search(Tcl_Interp* interp, int objc, Tcl_Obj* CONST objv[]) {
     int i;
     reg_registry* reg = registry_for(interp, reg_attached);
-    if (objc % 2 == 1) {
-        Tcl_WrongNumArgs(interp, 2, objv, "search ?key value ...?");
+    if ((objc > 2) && ((Tcl_GetString(objv[2])[0] == '-')
+                ? (objc % 2 == 0) : (objc % 2 == 1))) {
+        Tcl_WrongNumArgs(interp, 2, objv, "search ?options? ?key value ...?");
         return TCL_ERROR;
     } else if (reg == NULL) {
         return TCL_ERROR;
@@ -265,8 +294,24 @@
         reg_entry** entries;
         reg_error error;
         int entry_count;
+        int start;
+        int strategy;
+        /* try to use strategy */
+        if (objc > 2 && Tcl_GetString(objv[2])[0] == '-') {
+            int strat_index;
+            if (Tcl_GetIndexFromObjStruct(interp, objv[2], strategies,
+                        sizeof(strategy_type), "option", 0, &strat_index)
+                    == TCL_ERROR) {
+                return TCL_ERROR;
+            }
+            strategy = strategies[strat_index].strategy;
+            start = 3;
+        } else {
+            strategy = reg_strategy_exact;
+            start = 2;
+        }
         /* ensure that valid search keys were used */
-        for (i=2; i<objc; i+=2) {
+        for (i=start; i<objc; i+=2) {
             int index;
             if (Tcl_GetIndexFromObj(interp, objv[i], entry_props, "search key",
                         0, &index) != TCL_OK) {
@@ -276,20 +321,27 @@
         keys = malloc(key_count * sizeof(char*));
         vals = malloc(key_count * sizeof(char*));
         for (i=0; i<key_count; i+=1) {
-            keys[i] = Tcl_GetString(objv[2*i+2]);
-            vals[i] = Tcl_GetString(objv[2*i+3]);
+            keys[i] = Tcl_GetString(objv[2*i+start]);
+            vals[i] = Tcl_GetString(objv[2*i+start+1]);
         }
         entry_count = reg_entry_search(reg, keys, vals, key_count,
-                reg_strategy_equal, &entries, &error);
+                strategy, &entries, &error);
+        free(keys);
+        free(vals);
         if (entry_count >= 0) {
+            int retval;
             Tcl_Obj* resultObj;
             Tcl_Obj** objs;
-            list_entry_to_obj(interp, &objs, entries, entry_count,
-                        &error);
-            resultObj = Tcl_NewListObj(entry_count, objs);
-            Tcl_SetObjResult(interp, resultObj);
+            if (list_entry_to_obj(interp, &objs, entries, entry_count, &error)){
+                resultObj = Tcl_NewListObj(entry_count, objs);
+                Tcl_SetObjResult(interp, resultObj);
+                free(objs);
+                retval = TCL_OK;
+            } else {
+                retval = registry_failed(interp, &error);
+            }
             free(entries);
-            return TCL_OK;
+            return retval;
         }
         return registry_failed(interp, &error);
     }
@@ -360,6 +412,7 @@
             resultObj = Tcl_NewListObj(entry_count, objs);
             Tcl_SetObjResult(interp, resultObj);
             free(entries);
+            free(objs);
             return TCL_OK;
         }
         return registry_failed(interp, &error);
@@ -401,6 +454,7 @@
             resultObj = Tcl_NewListObj(entry_count, objs);
             Tcl_SetObjResult(interp, resultObj);
             free(entries);
+            free(objs);
             return TCL_OK;
         }
         return registry_failed(interp, &error);

Modified: trunk/base/src/registry2.0/entry.h
===================================================================
--- trunk/base/src/registry2.0/entry.h	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/registry2.0/entry.h	2007-08-06 20:13:30 UTC (rev 27528)
@@ -34,6 +34,8 @@
 
 #include <tcl.h>
 
+void delete_entry(ClientData clientData);
+
 int entry_cmd(ClientData clientData UNUSED, Tcl_Interp* interp, int objc,
         Tcl_Obj* CONST objv[]);
 

Modified: trunk/base/src/registry2.0/entryobj.c
===================================================================
--- trunk/base/src/registry2.0/entryobj.c	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/registry2.0/entryobj.c	2007-08-06 20:13:30 UTC (rev 27528)
@@ -74,7 +74,7 @@
             char* key = Tcl_GetString(objv[1]);
             char* value;
             reg_error error;
-            if (reg_entry_propget(reg, entry, key, &value, &error)) {
+            if (reg_entry_propget(entry, key, &value, &error)) {
                 Tcl_Obj* result = Tcl_NewStringObj(value, -1);
                 Tcl_SetObjResult(interp, result);
                 free(value);
@@ -94,7 +94,7 @@
             char* key = Tcl_GetString(objv[1]);
             char* value = Tcl_GetString(objv[2]);
             reg_error error;
-            if (reg_entry_propset(reg, entry, key, value, &error)) {
+            if (reg_entry_propset(entry, key, value, &error)) {
                 return TCL_OK;
             }
             return registry_failed(interp, &error);
@@ -125,15 +125,22 @@
         reg_error error;
         Tcl_Obj** listv;
         int listc;
+        int i;
+        int result = TCL_ERROR;
         if (Tcl_ListObjGetElements(interp, objv[2], &listc, &listv) != TCL_OK) {
             return TCL_ERROR;
         }
         if (list_obj_to_string(&files, listv, listc, &error)) {
-            if (reg_entry_map(reg, entry, files, listc, &error) == listc) {
-                return TCL_OK;
+            if (reg_entry_map(entry, files, listc, &error) == listc) {
+                result = TCL_OK;
+            } else {
+                result = registry_failed(interp, &error);
             }
+            free(files);
+        } else {
+            result = registry_failed(interp, &error);
         }
-        return registry_failed(interp, &error);
+        return result;
     }
 }
 
@@ -156,15 +163,22 @@
         reg_error error;
         Tcl_Obj** listv;
         int listc;
+        int i;
+        int result = TCL_ERROR;
         if (Tcl_ListObjGetElements(interp, objv[2], &listc, &listv) != TCL_OK) {
             return TCL_ERROR;
         }
         if (list_obj_to_string(&files, listv, listc, &error)) {
-            if (reg_entry_unmap(reg, entry, files, listc, &error) == listc) {
-                return TCL_OK;
+            if (reg_entry_unmap(entry, files, listc, &error) == listc) {
+                result = TCL_OK;
+            } else {
+                result = registry_failed(interp, &error);
             }
+            free(files);
+        } else {
+            result = registry_failed(interp, &error);
         }
-        return registry_failed(interp, &error);
+        return result;
     }
 }
 
@@ -179,23 +193,24 @@
     } else {
         char** files;
         reg_error error;
-        int file_count = reg_entry_files(reg, entry, &files, &error);
+        int file_count = reg_entry_files(entry, &files, &error);
         if (file_count >= 0) {
             int i;
             Tcl_Obj** objs;
+            int retval = TCL_ERROR;
             if (list_string_to_obj(&objs, files, file_count, &error)) {
                 Tcl_Obj* result = Tcl_NewListObj(file_count, objs);
                 Tcl_SetObjResult(interp, result);
-                for (i=0; i<file_count; i++) {
-                    free(files[i]);
-                }
-                free(files);
-                return TCL_OK;
+                free(objs);
+                retval = TCL_OK;
+            } else {
+                retval = registry_failed(interp, &error);
             }
             for (i=0; i<file_count; i++) {
                 free(files[i]);
             }
             free(files);
+            return retval;
         }
         return registry_failed(interp, &error);
     }

Modified: trunk/base/src/registry2.0/registry.c
===================================================================
--- trunk/base/src/registry2.0/registry.c	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/registry2.0/registry.c	2007-08-06 20:13:30 UTC (rev 27528)
@@ -31,15 +31,18 @@
 #endif
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <unistd.h>
 #include <tcl.h>
 
 #include <cregistry/registry.h>
 #include <cregistry/entry.h>
 
+#include "registry.h"
 #include "graph.h"
 #include "item.h"
 #include "entry.h"
+#include "entryobj.h"
 #include "util.h"
 
 int registry_failed(Tcl_Interp* interp, reg_error* errPtr) {
@@ -50,22 +53,44 @@
     return TCL_ERROR;
 }
 
+static void delete_entry_list(ClientData list, Tcl_Interp* interp UNUSED) {
+    entry_list* curr = *(entry_list**)list;
+    while (curr) {
+        entry_list* save = curr;
+        reg_entry_free(curr->entry);
+        curr = curr->next;
+        free(save);
+    }
+    *(entry_list**)list = NULL;
+}
+
+static void restore_entry_list(ClientData list, Tcl_Interp* interp) {
+    entry_list* curr = *(entry_list**)list;
+    while (curr) {
+        entry_list* save = curr;
+        Tcl_CreateObjCommand(interp, curr->entry->proc, entry_obj_cmd,
+                curr->entry, delete_entry);
+        curr = curr->next;
+        free(save);
+    }
+    *(entry_list**)list = NULL;
+}
+
 int registry_tcl_detach(Tcl_Interp* interp, reg_registry* reg,
         reg_error* errPtr) {
     reg_entry** entries;
-    int entry_count = reg_all_entries(reg, &entries, errPtr);
-    if (entry_count >= 0) {
-        int i;
-        for (i=0; i<entry_count; i++) {
-            if (entries[i]->proc) {
-                Tcl_DeleteCommand(interp, entries[i]->proc);
-            }
+    int entry_count = reg_all_open_entries(reg, &entries);
+    int i;
+    for (i=0; i<entry_count; i++) {
+        if (entries[i]->proc) {
+            Tcl_DeleteCommand(interp, entries[i]->proc);
         }
-        if (reg_detach(reg, errPtr)) {
-            return 1;
-        }
     }
-    return registry_failed(interp, errPtr);
+    free(entries);
+    if (!reg_detach(reg, errPtr)) {
+        return registry_failed(interp, errPtr);
+    }
+    return 1;
 }
 
 /**
@@ -216,28 +241,42 @@
         if (reg == NULL) {
             return TCL_ERROR;
         } else {
+            int result;
             reg_error error;
             if (reg_start_write(reg, &error)) {
-                int status = Tcl_EvalObjEx(interp, objv[1], 0);
-                switch (status) {
+                entry_list* list = NULL;
+                Tcl_SetAssocData(interp, "registry::deleted", delete_entry_list,
+                        &list);
+                result = Tcl_EvalObjEx(interp, objv[1], 0);
+                switch (result) {
                     case TCL_OK:
                         if (reg_commit(reg, &error)) {
-                            return TCL_OK;
+                            delete_entry_list(&list, interp);
+                        } else {
+                            result = registry_failed(interp, &error);
                         }
                         break;
                     case TCL_BREAK:
                         if (reg_rollback(reg, &error)) {
-                            return TCL_OK;
+                            restore_entry_list(&list, interp);
+                            result = TCL_OK;
+                        } else {
+                            result = registry_failed(interp, &error);
                         }
                         break;
                     default:
                         if (reg_rollback(reg, &error)) {
-                            return status;
+                            restore_entry_list(&list, interp);
+                        } else {
+                            result = registry_failed(interp, &error);
                         }
                         break;
                 }
+                Tcl_DeleteAssocData(interp, "registry::deleted");
+            } else {
+                result = registry_failed(interp, &error);
             }
-            return registry_failed(interp, &error);
+            return result;
         }
     }
 }

Modified: trunk/base/src/registry2.0/registry.h
===================================================================
--- trunk/base/src/registry2.0/registry.h	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/registry2.0/registry.h	2007-08-06 20:13:30 UTC (rev 27528)
@@ -36,6 +36,11 @@
 #include <sqlite3.h>
 #include <cregistry/entry.h>
 
+typedef struct _entry_list {
+    reg_entry* entry;
+    struct _entry_list* next;
+} entry_list;
+
 reg_registry* registry_for(Tcl_Interp* interp, int status);
 int registry_failed(Tcl_Interp* interp, reg_error* errPtr);
 

Modified: trunk/base/src/registry2.0/tests/entry.tcl
===================================================================
--- trunk/base/src/registry2.0/tests/entry.tcl	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/registry2.0/tests/entry.tcl	2007-08-06 20:13:30 UTC (rev 27528)
@@ -7,13 +7,14 @@
 
 	file delete [glob -nocomplain test.db*]
 
+    # can't create registry in some brain-dead place or in protected place
+    test_throws {registry::open /some/brain/dead/place} registry::cannot-init
+    test_throws {registry::open /etc/macports_test~} registry::cannot-init
+
     # can't use registry before it's opened
     test_throws {registry::write {}} registry::misuse
     registry::open test.db
 
-    test_throws {registry::entry create vim 7.1.000 0 {multibyte +} 0} \
-        registry::misuse
-
     # no nested transactions
     registry::write {
         test_throws {registry::read {}} registry::misuse
@@ -47,21 +48,31 @@
         test_equal {[$zlib revision]} 1
         test_equal {[$pcre variants]} {utf8 +}
     
+        # check that imaged and installed give correct results
+        # have to sort these because their orders aren't defined
         set imaged [registry::entry imaged]
+        test_equal {[lsort $imaged]} {[lsort "$vim1 $vim2 $vim3 $zlib $pcre"]}
+
         set installed [registry::entry installed]
+        test_equal {[lsort $installed]} {[lsort "$vim3 $zlib"]}
     }
 
-    # check that imaged and installed give correct results
-    # have to sort these because their orders aren't defined
-    test_equal {[lsort $imaged]} {[lsort "$vim1 $vim2 $vim3 $zlib $pcre"]}
-    test_equal {[lsort $installed]} {[lsort "$vim3 $zlib"]}
 
     # try searching for ports
+    # note that 7.1.2 != 7.1.002 but the VERSION collation should be smart
+    # enough to ignore the zeroes
+    set vim71002 [registry::entry search name vim version 7.1.2]
+    test_equal {[lsort $vim71002]} {[lsort "$vim2 $vim3"]}
+
     set no_variants [registry::entry search variants {}]
-    set vim71002 [registry::entry search name vim version 7.1.002]
     test_equal {[lsort $no_variants]} {[lsort "$vim2 $zlib"]}
-    test_equal {[lsort $vim71002]} {[lsort "$vim2 $vim3"]}
 
+    set vistar [registry::entry search -glob name vi*]
+    test_equal {[lsort $vistar]} {[lsort "$vim1 $vim2 $vim3"]}
+
+    set zlibpcre [registry::entry search -regexp name {zlib|pcre}]
+    test_equal {[lsort $zlibpcre]} {[lsort "$zlib $pcre"]}
+
     # try mapping files and checking their owners
     registry::write {
         $vim3 map [list /opt/local/bin/vim]
@@ -81,24 +92,23 @@
         test_equal {[registry::entry owner /opt/local/bin/vim]} {$vim3}
 
         # make sure you can't unmap a file you don't own
-        test_throws {$zlib unmap [list /opt/local/bin/vim]} \
-            registry::invalid
-        test_throws {$zlib unmap [list /opt/local/bin/emacs]} \
-            registry::invalid
+        test_throws {$zlib unmap [list /opt/local/bin/vim]} registry::invalid
+        test_throws {$zlib unmap [list /opt/local/bin/emacs]} registry::invalid
     }
 
-    # delete pcre
+    # try some deletions
+    test_equal {[registry::entry installed zlib]} {$zlib}
     test_equal {[registry::entry imaged pcre]} {$pcre}
-    # try rolling it back
+
+    # try rolling a deletion back
     registry::write {
-        registry::entry delete $pcre
+        registry::entry delete $zlib
         break
     }
-    test_equal {[registry::entry open pcre 7.1 1 {utf8 +} 0]} {$pcre}
-    # try actually deleting it
-    registry::write {
-        registry::entry delete $pcre
-    }
+    test_equal {[registry::entry open zlib 1.2.3 1 {} 0]} {$zlib}
+
+    # try actually deleting something
+    registry::entry delete $pcre
     test_throws {registry::entry open pcre 7.1 1 {utf8 +} 0} \
         registry::not-found
     test {![registry::entry exists $pcre]}

Modified: trunk/base/src/registry2.0/util.c
===================================================================
--- trunk/base/src/registry2.0/util.c	2007-08-06 20:13:19 UTC (rev 27527)
+++ trunk/base/src/registry2.0/util.c	2007-08-06 20:13:30 UTC (rev 27528)
@@ -234,23 +234,14 @@
 
 static int obj_to_string(void* userdata UNUSED, char** string, Tcl_Obj* obj,
         reg_error* errPtr UNUSED) {
-    int length;
-    char* value = Tcl_GetStringFromObj(obj, &length);
-    *string = malloc((length+1)*sizeof(char));
-    memcpy(*string, value, length);
-    (*string)[length] = '\0';
+    *string = Tcl_GetString(obj);
     return 1;
 }
 
-void free_string(void* userdata UNUSED, char* string) {
-    free(string);
-}
-
 int list_obj_to_string(char*** strings, const Tcl_Obj** objv, int objc,
         reg_error* errPtr) {
-    return recast(NULL, (cast_function*)obj_to_string,
-            (free_function*)free_string, (void***)strings, (void**)objv, objc,
-            errPtr);
+    return recast(NULL, (cast_function*)obj_to_string, NULL, (void***)strings,
+            (void**)objv, objc, errPtr);
 }
 
 static int string_to_obj(void* userdata UNUSED, Tcl_Obj** obj, char* string,
@@ -265,7 +256,6 @@
 
 int list_string_to_obj(Tcl_Obj*** objv, const char** strings, int objc,
         reg_error* errPtr) {
-    return recast(NULL, (cast_function*)string_to_obj,
-            (free_function*)free_obj, (void***)objv, (void**)strings, objc,
-            errPtr);
+    return recast(NULL, (cast_function*)string_to_obj, (free_function*)free_obj,
+            (void***)objv, (void**)strings, objc, errPtr);
 }

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.macosforge.org/pipermail/macports-changes/attachments/20070806/42a83f6f/attachment.html


More information about the macports-changes mailing list