* [PATCH 2/3] lib: encapsulate the use of notmuch_database_t field 'mode'
2020-07-26 23:31 simplify handling of writable database modes David Bremner
2020-07-26 23:31 ` [PATCH 1/3] lib: drop two gratuitous assignments to database mode David Bremner
@ 2020-07-26 23:31 ` David Bremner
2020-07-26 23:31 ` [PATCH 3/3] lib: replace use of static_cast for writable databases David Bremner
2020-07-27 16:58 ` simplify handling of writable database modes Tomi Ollila
3 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2020-07-26 23:31 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
The plan is to change the underlying representation.
---
lib/database.cc | 18 ++++++++++++------
lib/directory.cc | 2 +-
lib/message.cc | 4 ++--
lib/notmuch-private.h | 3 +++
4 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index b987cb42..08278235 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -69,6 +69,12 @@ _log_xapian_exception (const char *where, notmuch_database_t *notmuch, const Xa
notmuch->exception_reported = true;
}
+notmuch_database_mode_t
+_notmuch_database_mode (notmuch_database_t *notmuch)
+{
+ return notmuch->mode;
+}
+
/* Here's the current schema for our database (for NOTMUCH_DATABASE_VERSION):
*
* We currently have three different types of documents (mail, ghost,
@@ -783,7 +789,7 @@ notmuch_database_create_verbose (const char *path,
notmuch_status_t
_notmuch_database_ensure_writable (notmuch_database_t *notmuch)
{
- if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY) {
+ if (_notmuch_database_mode (notmuch) == NOTMUCH_DATABASE_MODE_READ_ONLY) {
_notmuch_database_log (notmuch, "Cannot write to a read-only database.\n");
return NOTMUCH_STATUS_READ_ONLY_DATABASE;
}
@@ -1107,7 +1113,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
* that transaction, or may discard committed (but
* unflushed) transactions. To be certain, explicitly
* cancel any outstanding transaction before closing. */
- if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE &&
+ if (_notmuch_database_mode (notmuch) == NOTMUCH_DATABASE_MODE_READ_WRITE &&
notmuch->atomic_nesting)
(static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))
->cancel_transaction ();
@@ -1130,7 +1136,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
notmuch_status_t
_notmuch_database_reopen (notmuch_database_t *notmuch)
{
- if (notmuch->mode != NOTMUCH_DATABASE_MODE_READ_ONLY)
+ if (_notmuch_database_mode (notmuch) != NOTMUCH_DATABASE_MODE_READ_ONLY)
return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
try {
@@ -1395,7 +1401,7 @@ notmuch_database_needs_upgrade (notmuch_database_t *notmuch)
{
unsigned int version;
- if (notmuch->mode != NOTMUCH_DATABASE_MODE_READ_WRITE)
+ if (_notmuch_database_mode (notmuch) != NOTMUCH_DATABASE_MODE_READ_WRITE)
return FALSE;
if (NOTMUCH_FEATURES_CURRENT & ~notmuch->features)
@@ -1697,7 +1703,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
notmuch_status_t
notmuch_database_begin_atomic (notmuch_database_t *notmuch)
{
- if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY ||
+ if (_notmuch_database_mode (notmuch) == NOTMUCH_DATABASE_MODE_READ_ONLY ||
notmuch->atomic_nesting > 0)
goto DONE;
@@ -1726,7 +1732,7 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch)
if (notmuch->atomic_nesting == 0)
return NOTMUCH_STATUS_UNBALANCED_ATOMIC;
- if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY ||
+ if (_notmuch_database_mode (notmuch) == NOTMUCH_DATABASE_MODE_READ_ONLY ||
notmuch->atomic_nesting > 1)
goto DONE;
diff --git a/lib/directory.cc b/lib/directory.cc
index 044cd680..9ad66e82 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -127,7 +127,7 @@ _notmuch_directory_find_or_create (notmuch_database_t *notmuch,
path = _notmuch_database_relative_path (notmuch, path);
- if (create && notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+ if (create && _notmuch_database_mode (notmuch) == NOTMUCH_DATABASE_MODE_READ_ONLY)
INTERNAL_ERROR ("Failure to ensure database is writable");
directory = talloc (notmuch, notmuch_directory_t);
diff --git a/lib/message.cc b/lib/message.cc
index 64798413..d23e64ab 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -275,7 +275,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch,
return NULL;
}
- if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+ if (_notmuch_database_mode (notmuch) == NOTMUCH_DATABASE_MODE_READ_ONLY)
INTERNAL_ERROR ("Failure to ensure database is writable.");
try {
@@ -1324,7 +1324,7 @@ _notmuch_message_sync (notmuch_message_t *message)
{
Xapian::WritableDatabase *db;
- if (message->notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+ if (_notmuch_database_mode (message->notmuch) == NOTMUCH_DATABASE_MODE_READ_ONLY)
return;
if (! message->modified)
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 2bbbb293..57ec7f72 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -259,6 +259,9 @@ _notmuch_directory_find_or_create (notmuch_database_t *notmuch,
unsigned int
_notmuch_directory_get_document_id (notmuch_directory_t *directory);
+notmuch_database_mode_t
+_notmuch_database_mode (notmuch_database_t *notmuch);
+
/* message.cc */
notmuch_message_t *
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] lib: replace use of static_cast for writable databases
2020-07-26 23:31 simplify handling of writable database modes David Bremner
2020-07-26 23:31 ` [PATCH 1/3] lib: drop two gratuitous assignments to database mode David Bremner
2020-07-26 23:31 ` [PATCH 2/3] lib: encapsulate the use of notmuch_database_t field 'mode' David Bremner
@ 2020-07-26 23:31 ` David Bremner
2020-07-27 16:58 ` simplify handling of writable database modes Tomi Ollila
3 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2020-07-26 23:31 UTC (permalink / raw)
To: notmuch; +Cc: David Bremner
static_cast is a bit tricky to understand and error prone, so add a
second pointer to (potentially the same) Xapian database object that
we know has the right subclass.
---
lib/add-message.cc | 13 +++----------
lib/config.cc | 4 +---
lib/database-private.h | 2 +-
lib/database.cc | 21 ++++++++++++---------
lib/directory.cc | 18 +++++++-----------
lib/message.cc | 10 +++-------
6 files changed, 27 insertions(+), 41 deletions(-)
diff --git a/lib/add-message.cc b/lib/add-message.cc
index 9dd4b697..485debad 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -43,15 +43,12 @@ _notmuch_database_generate_thread_id (notmuch_database_t *notmuch)
/* 16 bytes (+ terminator) for hexadecimal representation of
* a 64-bit integer. */
static char thread_id[17];
- Xapian::WritableDatabase *db;
-
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
notmuch->last_thread_id++;
sprintf (thread_id, "%016" PRIx64, notmuch->last_thread_id);
- db->set_metadata ("last_thread_id", thread_id);
+ notmuch->writable_xapian_db->set_metadata ("last_thread_id", thread_id);
return thread_id;
}
@@ -161,7 +158,7 @@ _resolve_message_id_to_thread_id_old (notmuch_database_t *notmuch,
* can return the thread ID stored in the metadata. Otherwise, we
* generate a new thread ID and store it there.
*/
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
+ db = notmuch->writable_xapian_db;
metadata_key = _get_metadata_thread_id_key (ctx, message_id);
thread_id_string = notmuch->xapian_db->get_metadata (metadata_key);
@@ -370,13 +367,9 @@ _consume_metadata_thread_id (void *ctx, notmuch_database_t *notmuch,
if (stored_id.empty ()) {
return NULL;
} else {
- Xapian::WritableDatabase *db;
-
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
-
/* Clear the metadata for this message ID. We don't need it
* anymore. */
- db->set_metadata (metadata_key, "");
+ notmuch->writable_xapian_db->set_metadata (metadata_key, "");
return talloc_strdup (ctx, stored_id.c_str ());
}
diff --git a/lib/config.cc b/lib/config.cc
index 20036471..dae0ff0e 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -45,15 +45,13 @@ notmuch_database_set_config (notmuch_database_t *notmuch,
const char *value)
{
notmuch_status_t status;
- Xapian::WritableDatabase *db;
status = _notmuch_database_ensure_writable (notmuch);
if (status)
return status;
try {
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
- db->set_metadata (CONFIG_PREFIX + key, value);
+ notmuch->writable_xapian_db->set_metadata (CONFIG_PREFIX + key, value);
} catch (const Xapian::Error &error) {
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
notmuch->exception_reported = true;
diff --git a/lib/database-private.h b/lib/database-private.h
index d2c25313..041602cd 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -189,11 +189,11 @@ struct _notmuch_database {
char *path;
- notmuch_database_mode_t mode;
int atomic_nesting;
/* true if changes have been made in this atomic section */
bool atomic_dirty;
Xapian::Database *xapian_db;
+ Xapian::WritableDatabase *writable_xapian_db;
bool open;
/* Bit mask of features used by this database. This is a
* bitwise-OR of NOTMUCH_FEATURE_* values (above). */
diff --git a/lib/database.cc b/lib/database.cc
index 08278235..75189685 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -72,7 +72,10 @@ _log_xapian_exception (const char *where, notmuch_database_t *notmuch, const Xa
notmuch_database_mode_t
_notmuch_database_mode (notmuch_database_t *notmuch)
{
- return notmuch->mode;
+ if (notmuch->writable_xapian_db)
+ return NOTMUCH_DATABASE_MODE_READ_WRITE;
+ else
+ return NOTMUCH_DATABASE_MODE_READ_ONLY;
}
/* Here's the current schema for our database (for NOTMUCH_DATABASE_VERSION):
@@ -976,7 +979,7 @@ notmuch_database_open_verbose (const char *path,
strip_trailing (notmuch->path, '/');
- notmuch->mode = mode;
+ notmuch->writable_xapian_db = NULL;
notmuch->atomic_nesting = 0;
notmuch->view = 1;
try {
@@ -984,8 +987,9 @@ notmuch_database_open_verbose (const char *path,
string last_mod;
if (mode == NOTMUCH_DATABASE_MODE_READ_WRITE) {
- notmuch->xapian_db = new Xapian::WritableDatabase (xapian_path,
- DB_ACTION);
+ notmuch->writable_xapian_db = new Xapian::WritableDatabase (xapian_path,
+ DB_ACTION);
+ notmuch->xapian_db = notmuch->writable_xapian_db;
} else {
notmuch->xapian_db = new Xapian::Database (xapian_path);
}
@@ -1115,8 +1119,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
* cancel any outstanding transaction before closing. */
if (_notmuch_database_mode (notmuch) == NOTMUCH_DATABASE_MODE_READ_WRITE &&
notmuch->atomic_nesting)
- (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))
- ->cancel_transaction ();
+ notmuch->writable_xapian_db->cancel_transaction ();
/* Close the database. This implicitly flushes
* outstanding changes. */
@@ -1454,7 +1457,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
if (status)
return status;
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
+ db = notmuch->writable_xapian_db;
target_features = notmuch->features | NOTMUCH_FEATURES_CURRENT;
new_features = NOTMUCH_FEATURES_CURRENT & ~notmuch->features;
@@ -1711,7 +1714,7 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch)
return NOTMUCH_STATUS_UPGRADE_REQUIRED;
try {
- (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false);
+ notmuch->writable_xapian_db->begin_transaction (false);
} catch (const Xapian::Error &error) {
_notmuch_database_log (notmuch, "A Xapian exception occurred beginning transaction: %s.\n",
error.get_msg ().c_str ());
@@ -1736,7 +1739,7 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch)
notmuch->atomic_nesting > 1)
goto DONE;
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
+ db = notmuch->writable_xapian_db;
try {
db->commit_transaction ();
diff --git a/lib/directory.cc b/lib/directory.cc
index 9ad66e82..79ceea31 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -112,7 +112,6 @@ _notmuch_directory_find_or_create (notmuch_database_t *notmuch,
notmuch_find_flags_t flags,
notmuch_status_t *status_ret)
{
- Xapian::WritableDatabase *db;
notmuch_directory_t *directory;
notmuch_private_status_t private_status;
const char *db_path;
@@ -189,10 +188,10 @@ _notmuch_directory_find_or_create (notmuch_database_t *notmuch,
directory->doc.add_value (NOTMUCH_VALUE_TIMESTAMP,
Xapian::sortable_serialise (0));
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
-
directory->document_id = _notmuch_database_generate_doc_id (notmuch);
- db->replace_document (directory->document_id, directory->doc);
+ directory->notmuch->
+ writable_xapian_db
+ -> replace_document (directory->document_id, directory->doc);
talloc_free (local);
}
@@ -226,20 +225,18 @@ notmuch_directory_set_mtime (notmuch_directory_t *directory,
time_t mtime)
{
notmuch_database_t *notmuch = directory->notmuch;
- Xapian::WritableDatabase *db;
notmuch_status_t status;
status = _notmuch_database_ensure_writable (notmuch);
if (status)
return status;
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
-
try {
directory->doc.add_value (NOTMUCH_VALUE_TIMESTAMP,
Xapian::sortable_serialise (mtime));
- db->replace_document (directory->document_id, directory->doc);
+ directory->notmuch
+ ->writable_xapian_db->replace_document (directory->document_id, directory->doc);
directory->mtime = mtime;
@@ -309,15 +306,14 @@ notmuch_status_t
notmuch_directory_delete (notmuch_directory_t *directory)
{
notmuch_status_t status;
- Xapian::WritableDatabase *db;
status = _notmuch_database_ensure_writable (directory->notmuch);
if (status)
return status;
try {
- db = static_cast <Xapian::WritableDatabase *> (directory->notmuch->xapian_db);
- db->delete_document (directory->document_id);
+ directory->notmuch->
+ writable_xapian_db->delete_document (directory->document_id);
} catch (const Xapian::Error &error) {
_notmuch_database_log (directory->notmuch,
"A Xapian exception occurred deleting directory entry: %s.\n",
diff --git a/lib/message.cc b/lib/message.cc
index d23e64ab..fca99082 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1322,8 +1322,6 @@ _notmuch_message_upgrade_last_mod (notmuch_message_t *message)
void
_notmuch_message_sync (notmuch_message_t *message)
{
- Xapian::WritableDatabase *db;
-
if (_notmuch_database_mode (message->notmuch) == NOTMUCH_DATABASE_MODE_READ_ONLY)
return;
@@ -1342,8 +1340,8 @@ _notmuch_message_sync (notmuch_message_t *message)
_notmuch_database_new_revision (
message->notmuch)));
- db = static_cast <Xapian::WritableDatabase *> (message->notmuch->xapian_db);
- db->replace_document (message->doc_id, message->doc);
+ message->notmuch->writable_xapian_db->
+ replace_document (message->doc_id, message->doc);
message->modified = false;
}
@@ -1353,7 +1351,6 @@ notmuch_status_t
_notmuch_message_delete (notmuch_message_t *message)
{
notmuch_status_t status;
- Xapian::WritableDatabase *db;
const char *mid, *tid, *query_string;
notmuch_message_t *ghost;
notmuch_private_status_t private_status;
@@ -1370,8 +1367,7 @@ _notmuch_message_delete (notmuch_message_t *message)
if (status)
return status;
- db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
- db->delete_document (message->doc_id);
+ message->notmuch->writable_xapian_db->delete_document (message->doc_id);
/* if this was a ghost to begin with, we are done */
private_status = _notmuch_message_has_term (message, "type", "ghost", &is_ghost);
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread