There is a need for setting options before opening a database, such as setting a logging function to use instead of writing to stdout or stderr. It would be possible to do this by adding new parameters to notmuch_database_create and notmuch_database_open, but maintaining a backwards compatible API and ABI when new options are added becomes burdensome. Instead, split the opaque database object creation from notmuch_database_create and notmuch_database_open into a new notmuch_database_new call, to allow operations on the handle before create and open. This creates API and ABI breakage now, but allows easier future extensions. The notmuch_database_new call becomes a natural pair to the already existing notmuch_database_destroy, and it should be possible to call open/close multiple times using an initialized handle. --- lib/database.cc | 64 ++++++++++++++++++++++++++++------------------------ lib/notmuch.h | 52 ++++++++++++++++++++++++++++++++---------- notmuch-compact.c | 11 ++++++++- notmuch-count.c | 10 ++++++-- notmuch-dump.c | 10 ++++++-- notmuch-insert.c | 10 ++++++-- notmuch-new.c | 14 +++++++----- notmuch-reply.c | 10 ++++++-- notmuch-restore.c | 10 ++++++-- notmuch-search.c | 10 ++++++-- notmuch-show.c | 10 ++++++-- notmuch-tag.c | 10 ++++++-- test/random-corpus.c | 10 ++++++-- test/symbol-test.cc | 3 ++- 14 files changed, 166 insertions(+), 68 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 98e2c31..386b93a 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -539,10 +539,21 @@ parse_references (void *ctx, } notmuch_status_t -notmuch_database_create (const char *path, notmuch_database_t **database) +notmuch_database_new (notmuch_database_t **notmuch) +{ + /* Note: try to avoid error conditions! No error printing! */ + + *notmuch = talloc_zero (NULL, notmuch_database_t); + if (! *notmuch) + return NOTMUCH_STATUS_OUT_OF_MEMORY; + + return NOTMUCH_STATUS_SUCCESS; +} + +notmuch_status_t +notmuch_database_create (notmuch_database_t *notmuch, const char *path) { notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; - notmuch_database_t *notmuch = NULL; char *notmuch_path = NULL; struct stat st; int err; @@ -579,25 +590,18 @@ notmuch_database_create (const char *path, notmuch_database_t **database) goto DONE; } - status = notmuch_database_open (path, - NOTMUCH_DATABASE_MODE_READ_WRITE, - ¬much); + status = notmuch_database_open (notmuch, path, + NOTMUCH_DATABASE_MODE_READ_WRITE); if (status) goto DONE; status = notmuch_database_upgrade (notmuch, NULL, NULL); - if (status) { + if (status) notmuch_database_close(notmuch); - notmuch = NULL; - } DONE: if (notmuch_path) talloc_free (notmuch_path); - if (database) - *database = notmuch; - else - talloc_free (notmuch); return status; } @@ -612,14 +616,15 @@ _notmuch_database_ensure_writable (notmuch_database_t *notmuch) return NOTMUCH_STATUS_SUCCESS; } +/* + * XXX: error handling should clean up *all* state created! + */ notmuch_status_t -notmuch_database_open (const char *path, - notmuch_database_mode_t mode, - notmuch_database_t **database) +notmuch_database_open (notmuch_database_t *notmuch, const char *path, + notmuch_database_mode_t mode) { notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; void *local = talloc_new (NULL); - notmuch_database_t *notmuch = NULL; char *notmuch_path, *xapian_path; struct stat st; int err; @@ -663,7 +668,6 @@ notmuch_database_open (const char *path, initialized = 1; } - notmuch = talloc_zero (NULL, notmuch_database_t); notmuch->exception_reported = FALSE; notmuch->path = talloc_strdup (notmuch, path); @@ -689,8 +693,7 @@ notmuch_database_open (const char *path, " read-write mode.\n", notmuch_path, version, NOTMUCH_DATABASE_VERSION); notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; - notmuch_database_destroy (notmuch); - notmuch = NULL; + notmuch_database_close (notmuch); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } @@ -752,21 +755,19 @@ notmuch_database_open (const char *path, } catch (const Xapian::Error &error) { fprintf (stderr, "A Xapian exception occurred opening database: %s\n", error.get_msg().c_str()); - notmuch_database_destroy (notmuch); - notmuch = NULL; + notmuch_database_close (notmuch); status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; } DONE: talloc_free (local); - if (database) - *database = notmuch; - else - talloc_free (notmuch); return status; } +/* + * XXX: close should clean up *all* state created by open/create! + */ notmuch_status_t notmuch_database_close (notmuch_database_t *notmuch) { @@ -869,7 +870,8 @@ public: * compaction process to protect data integrity. */ notmuch_status_t -notmuch_database_compact (const char *path, +notmuch_database_compact (notmuch_database_t *notmuch, + const char *path, const char *backup_path, notmuch_compact_status_cb_t status_cb, void *closure) @@ -877,7 +879,6 @@ notmuch_database_compact (const char *path, void *local; char *notmuch_path, *xapian_path, *compact_xapian_path; notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS; - notmuch_database_t *notmuch = NULL; struct stat statbuf; notmuch_bool_t keep_backup; @@ -885,7 +886,8 @@ notmuch_database_compact (const char *path, if (! local) return NOTMUCH_STATUS_OUT_OF_MEMORY; - ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much); + ret = notmuch_database_open (notmuch, path, + NOTMUCH_DATABASE_MODE_READ_WRITE); if (ret) { goto DONE; } @@ -971,8 +973,9 @@ notmuch_database_compact (const char *path, } DONE: + /* XXX: error handling */ if (notmuch) - ret = notmuch_database_destroy (notmuch); + ret = notmuch_database_close (notmuch); talloc_free (local); @@ -980,7 +983,8 @@ notmuch_database_compact (const char *path, } #else notmuch_status_t -notmuch_database_compact (unused (const char *path), +notmuch_database_compact (unused (notmuch_database_t *notmuch), + unused (const char *path), unused (const char *backup_path), unused (notmuch_compact_status_cb_t status_cb), unused (void *closure)) diff --git a/lib/notmuch.h b/lib/notmuch.h index dbdce86..cd58d15 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -149,6 +149,28 @@ typedef struct _notmuch_tags notmuch_tags_t; typedef struct _notmuch_directory notmuch_directory_t; typedef struct _notmuch_filenames notmuch_filenames_t; +/* Initialize a new, empty database handle. + * + * The database handle is required for creating, opening, and + * compacting a database. For further database operations, the + * database needs to be created or opened. + * + * After a successful call to notmuch_database_new, the returned + * database handle will remain in memory, so the caller should call + * notmuch_database_destroy when finished with the database handle. + * + * In case of any failure, this function returns an error status and + * sets *notmuch to NULL. + * + * Return value: + * + * NOTMUCH_STATUS_SUCCESS: Successfully created the database object. + * + * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory. + */ +notmuch_status_t +notmuch_database_new (notmuch_database_t **notmuch); + /* Create a new, empty notmuch database located at 'path'. * * The path should be a top-level directory to a collection of @@ -156,9 +178,9 @@ typedef struct _notmuch_filenames notmuch_filenames_t; * create a new ".notmuch" directory within 'path' where notmuch will * store its data. * - * After a successful call to notmuch_database_create, the returned - * database will be open so the caller should call - * notmuch_database_destroy when finished with it. + * After a successful call to notmuch_database_create, the database + * will be open so the caller should call notmuch_database_close (or + * notmuch_database_destroy) when finished with the database. * * The database will not yet have any data in it * (notmuch_database_create itself is a very cheap function). Messages @@ -166,7 +188,8 @@ typedef struct _notmuch_filenames notmuch_filenames_t; * notmuch_database_add_message. * * In case of any failure, this function returns an error status and - * sets *database to NULL (after printing an error message on stderr). + * the database will be closed (after printing an error message on + * stderr). * * Return value: * @@ -183,7 +206,7 @@ typedef struct _notmuch_filenames notmuch_filenames_t; * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred. */ notmuch_status_t -notmuch_database_create (const char *path, notmuch_database_t **database); +notmuch_database_create (notmuch_database_t *notmuch, const char *path); typedef enum { NOTMUCH_DATABASE_MODE_READ_ONLY = 0, @@ -201,11 +224,13 @@ typedef enum { * An existing notmuch database can be identified by the presence of a * directory named ".notmuch" below 'path'. * - * The caller should call notmuch_database_destroy when finished with - * this database. + * After a successful call to notmuch_database_open, the database will + * be open so the caller should call notmuch_database_close (or + * notmuch_database_destroy) when finished with the database. * * In case of any failure, this function returns an error status and - * sets *database to NULL (after printing an error message on stderr). + * the database will be closed (after printing an error message on + * stderr). * * Return value: * @@ -222,9 +247,8 @@ typedef enum { * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred. */ notmuch_status_t -notmuch_database_open (const char *path, - notmuch_database_mode_t mode, - notmuch_database_t **database); +notmuch_database_open (notmuch_database_t *notmuch, const char *path, + notmuch_database_mode_t mode); /* Close the given notmuch database. * @@ -264,7 +288,8 @@ typedef void (*notmuch_compact_status_cb_t)(const char *message, void *closure); * 'closure' is passed verbatim to any callback invoked. */ notmuch_status_t -notmuch_database_compact (const char* path, +notmuch_database_compact (notmuch_database_t *notmuch, + const char* path, const char* backup_path, notmuch_compact_status_cb_t status_cb, void *closure); @@ -272,6 +297,9 @@ notmuch_database_compact (const char* path, /* Destroy the notmuch database, closing it if necessary and freeing * all associated resources. * + * A database handle initialized with notmuch_database_new should be + * destroyed by calling notmuch_database_destroy. + * * Return value as in notmuch_database_close if the database was open; * notmuch_database_destroy itself has no failure modes. */ diff --git a/notmuch-compact.c b/notmuch-compact.c index 8b820c0..626685e 100644 --- a/notmuch-compact.c +++ b/notmuch-compact.c @@ -29,6 +29,7 @@ status_update_cb (const char *msg, unused (void *closure)) int notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[]) { + notmuch_database_t *notmuch = NULL; const char *path = notmuch_config_get_database_path (config); const char *backup_path = NULL; notmuch_status_t ret; @@ -46,7 +47,13 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[]) if (! quiet) printf ("Compacting database...\n"); - ret = notmuch_database_compact (path, backup_path, + + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + ret = notmuch_database_compact (notmuch, path, backup_path, quiet ? NULL : status_update_cb, NULL); if (ret) { fprintf (stderr, "Compaction failed: %s\n", notmuch_status_to_string (ret)); @@ -60,5 +67,7 @@ notmuch_compact_command (notmuch_config_t *config, int argc, char *argv[]) printf ("Done.\n"); } + notmuch_database_destroy (notmuch); + return 0; } diff --git a/notmuch-count.c b/notmuch-count.c index 01e4e30..15c95c7 100644 --- a/notmuch-count.c +++ b/notmuch-count.c @@ -170,8 +170,14 @@ notmuch_count_command (notmuch_config_t *config, int argc, char *argv[]) return 1; } - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY)) return 1; query_str = query_string_from_args (config, argc-opt_index, argv+opt_index); diff --git a/notmuch-dump.c b/notmuch-dump.c index 2024e30..73579bc 100644 --- a/notmuch-dump.c +++ b/notmuch-dump.c @@ -33,8 +33,14 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[]) notmuch_tags_t *tags; const char *query_str = ""; - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY)) return 1; char *output_file_name = NULL; diff --git a/notmuch-insert.c b/notmuch-insert.c index 2207b1e..4a8aad3 100644 --- a/notmuch-insert.c +++ b/notmuch-insert.c @@ -467,8 +467,14 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[]) action.sa_flags = 0; sigaction (SIGINT, &action, NULL); - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_WRITE)) return 1; ret = insert_message (config, notmuch, STDIN_FILENO, maildir, tag_ops); diff --git a/notmuch-new.c b/notmuch-new.c index ba05cb4..f72a4de 100644 --- a/notmuch-new.c +++ b/notmuch-new.c @@ -914,6 +914,11 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) return ret; } + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + dot_notmuch_path = talloc_asprintf (config, "%s/%s", db_path, ".notmuch"); if (stat (dot_notmuch_path, &st)) { @@ -925,12 +930,12 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) return 1; printf ("Found %d total files (that's not much mail).\n", count); - if (notmuch_database_create (db_path, ¬much)) + if (notmuch_database_create (notmuch, db_path)) return 1; add_files_state.total_files = count; } else { - if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, - ¬much)) + if (notmuch_database_open (notmuch, db_path, + NOTMUCH_DATABASE_MODE_READ_WRITE)) return 1; if (notmuch_database_needs_upgrade (notmuch)) { @@ -945,9 +950,6 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) add_files_state.total_files = 0; } - if (notmuch == NULL) - return 1; - /* Setup our handler for SIGINT. We do this after having * potentially done a database upgrade we this interrupt handler * won't support. */ diff --git a/notmuch-reply.c b/notmuch-reply.c index 9d6f843..7b80841 100644 --- a/notmuch-reply.c +++ b/notmuch-reply.c @@ -769,8 +769,14 @@ notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[]) return 1; } - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY)) return 1; query = notmuch_query_create (notmuch, query_string); diff --git a/notmuch-restore.c b/notmuch-restore.c index 1419621..fc37838 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -138,8 +138,14 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[]) int opt_index; int input_format = DUMP_FORMAT_AUTO; - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_WRITE)) return 1; if (notmuch_config_get_maildir_synchronize_flags (config)) diff --git a/notmuch-search.c b/notmuch-search.c index 7c973b3..50aace9 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -430,8 +430,14 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[]) notmuch_exit_if_unsupported_format (); - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY)) return 1; query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index); diff --git a/notmuch-show.c b/notmuch-show.c index c07f887..bc44b2c 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1201,8 +1201,14 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[]) return 1; } - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY)) return 1; query = notmuch_query_create (notmuch, query_string); diff --git a/notmuch-tag.c b/notmuch-tag.c index 3b09df9..6e29799 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -254,8 +254,14 @@ notmuch_tag_command (notmuch_config_t *config, int argc, char *argv[]) } } - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_WRITE)) return 1; if (notmuch_config_get_maildir_synchronize_flags (config)) diff --git a/test/random-corpus.c b/test/random-corpus.c index 790193d..2b205e5 100644 --- a/test/random-corpus.c +++ b/test/random-corpus.c @@ -164,8 +164,14 @@ main (int argc, char **argv) if (config == NULL) return 1; - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much)) + if (notmuch_database_new (¬much)) { + fprintf (stderr, "Out of memory\n"); + return 1; + } + + if (notmuch_database_open (notmuch, + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_WRITE)) return 1; srandom (seed); diff --git a/test/symbol-test.cc b/test/symbol-test.cc index 3e96c03..47c5351 100644 --- a/test/symbol-test.cc +++ b/test/symbol-test.cc @@ -5,7 +5,8 @@ int main() { notmuch_database_t *notmuch; - notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much); + notmuch_database_new (¬much); + notmuch_database_open (notmuch, "fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY); try { (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN); -- 1.8.4.2