[PATCH] lib: another iterator-temporary/stale-pointer bug
Tamas Szakaly points out [1] that the bug fixed in 51b073c still exists in at least one place. This change follows the suggestion of [2] and creates a block scope temporary std::string to avoid the rules of iterators temporaries. [1]: id:20141226113755.GA64154@pamparam [2]: id:20141226230655.GA41992@pamparam --- I decided to take a more minimalist approach than [2]. In particular using direntry for two different things seemed slightly trickier than necessary, for no obvious performance gain (calling .c_str() should be cheap). lib/message.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index a7a13cc..bacb4d4 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -641,15 +641,16 @@ _notmuch_message_add_directory_terms (void *ctx, notmuch_message_t *message) unsigned int directory_id; const char *direntry, *directory; char *colon; + const std::string term = *i; /* Terminate loop at first term without desired prefix. */ - if (strncmp ((*i).c_str (), direntry_prefix, direntry_prefix_len)) + if (strncmp (term.c_str (), direntry_prefix, direntry_prefix_len)) break; /* Indicate that there are filenames remaining. */ status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID; - direntry = (*i).c_str (); + direntry = term.c_str (); direntry += direntry_prefix_len; directory_id = strtol (direntry, colon, 10); -- 2.1.3 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
libnotmuch logging overhaul v3
changes since the last round [1]
- remove unused variable
- fix the internal usage of n_d_open to use _verbose version
- revert the hiding of query debugging.
[1]: id:1419717937-1108-2-git-send-email-da...@tethera.net
diff --git a/lib/database.cc b/lib/database.cc
index 16cd940..b144885 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -377,7 +377,6 @@ _notmuch_database_log (notmuch_database_t *notmuch,
...)
{
va_list va_args;
-const char *message;
va_start (va_args, format);
@@ -693,9 +692,9 @@ notmuch_database_create_verbose (const char *path,
goto DONE;
}
-status = notmuch_database_open (path,
- NOTMUCH_DATABASE_MODE_READ_WRITE,
- notmuch);
+status = notmuch_database_open_verbose (path,
+ NOTMUCH_DATABASE_MODE_READ_WRITE,
+ notmuch, message);
if (status)
goto DONE;
@@ -1101,13 +1100,18 @@ notmuch_database_compact (const char *path,
notmuch_database_t *notmuch = NULL;
struct stat statbuf;
notmuch_bool_t keep_backup;
+char *message = NULL;
local = talloc_new (NULL);
if (! local)
return NOTMUCH_STATUS_OUT_OF_MEMORY;
-ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE,
notmuch);
+ret = notmuch_database_open_verbose (path,
+NOTMUCH_DATABASE_MODE_READ_WRITE,
+notmuch,
+message);
if (ret) {
+ if (status_cb) status_cb (message, closure);
goto DONE;
}
diff --git a/lib/query.cc b/lib/query.cc
index 2e20ab2..9ed420c 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -78,7 +78,7 @@ notmuch_query_create (notmuch_database_t *notmuch,
notmuch_query_t *query;
if (_debug_query ())
- _notmuch_database_log (notmuch, Query string is:\n%s\n, query_string);
+ fprintf (stderr, Query string is:\n%s\n, query_string);
query = talloc (notmuch, notmuch_query_t);
if (unlikely (query == NULL))
@@ -266,9 +266,9 @@ notmuch_query_search_messages (notmuch_query_t *query)
}
if (_debug_query ()) {
- _notmuch_database_log (notmuch, Exclude query is:\n%s\n,
+ fprintf (stderr, Exclude query is:\n%s\n,
exclude_query.get_description ().c_str ());
- _notmuch_database_log (notmuch, Final query is:\n%s\n,
+ fprintf (stderr, Final query is:\n%s\n,
final_query.get_description ().c_str ());
}
@@ -549,9 +549,9 @@ notmuch_query_count_messages (notmuch_query_t *query)
enquire.set_docid_order(Xapian::Enquire::ASCENDING);
if (_debug_query ()) {
- _notmuch_database_log (notmuch, Exclude query is:\n%s\n,
+ fprintf (stderr, Exclude query is:\n%s\n,
exclude_query.get_description ().c_str ());
- _notmuch_database_log (notmuch, Final query is:\n%s\n,
+ fprintf (stderr, Final query is:\n%s\n,
final_query.get_description ().c_str ());
}
lib/database.cc | 14 +-
lib/query.cc| 10 +-
2 files changed, 14 insertions(+), 10 deletions(-)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v3 4/6] lib: add private function to extract the database for a message.
This is needed by logging in functions outside message.cc that take
only a notmuch_message_t object.
---
lib/message.cc| 6 ++
lib/notmuch-private.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/lib/message.cc b/lib/message.cc
index a7a13cc..43cc078 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1625,3 +1625,9 @@ notmuch_message_destroy (notmuch_message_t *message)
{
talloc_free (message);
}
+
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message)
+{
+return message-notmuch;
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 7c6cfc0..fad3a92 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -473,6 +473,8 @@ _notmuch_doc_id_set_remove (notmuch_doc_id_set_t *doc_ids,
void
_notmuch_message_add_reply (notmuch_message_t *message,
notmuch_message_t *reply);
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message);
/* sha1.c */
--
2.1.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v3 2/6] lib/database: add field for last error string
The idea is to have a logging function setting this string instead of
printing to stderr.
---
lib/database-private.h | 4
lib/database.cc| 7 +++
lib/notmuch.h | 7 +++
3 files changed, 18 insertions(+)
diff --git a/lib/database-private.h b/lib/database-private.h
index 15e03cc..7efd98b 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -146,6 +146,10 @@ struct _notmuch_database {
unsigned int last_doc_id;
uint64_t last_thread_id;
+/* error reporting; this value persists only until the
+ * next library call. May be NULL */
+char *status_string;
+
Xapian::QueryParser *query_parser;
Xapian::TermGenerator *term_gen;
Xapian::ValueRangeProcessor *value_range_processor;
diff --git a/lib/database.cc b/lib/database.cc
index 4f4e871..18db902 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -858,6 +858,7 @@ notmuch_database_open_verbose (const char *path,
notmuch = talloc_zero (NULL, notmuch_database_t);
notmuch-exception_reported = FALSE;
+notmuch-status_string = NULL;
notmuch-path = talloc_strdup (notmuch, path);
if (notmuch-path[strlen (notmuch-path) - 1] == '/')
@@ -2543,3 +2544,9 @@ notmuch_database_get_all_tags (notmuch_database_t *db)
return NULL;
}
}
+
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch)
+{
+return notmuch-status_string;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0dfac8f..2ab2998 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -302,6 +302,13 @@ notmuch_database_open_verbose (const char *path,
char **error_message);
/**
+ * Retrieve last status string for given database.
+ *
+ */
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch);
+
+/**
* Commit changes and close the given notmuch database.
*
* After notmuch_database_close has been called, calls to other
--
2.1.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v3 6/6] lib: eliminate fprintf from _notmuch_message_file_open
You may wonder why _notmuch_message_file_open_ctx has two parameters.
This is because we need sometime to use a ctx which is a
notmuch_message_t. While we could get the database from this, there is
no easy way in C to tell type we are getting. The remaining fprintf is
removed by Jani's series un-deprecating single message mboxes.
---
lib/database.cc | 2 +-
lib/message-file.c| 11 +++
lib/message.cc| 3 ++-
lib/notmuch-private.h | 5 +++--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 19f5f0e..b144885 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2298,7 +2298,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
if (ret)
return ret;
-message_file = _notmuch_message_file_open (filename);
+message_file = _notmuch_message_file_open (notmuch, filename);
if (message_file == NULL)
return NOTMUCH_STATUS_FILE_ERROR;
diff --git a/lib/message-file.c b/lib/message-file.c
index eda1b74..1f55cdf 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -76,7 +76,8 @@ _notmuch_message_file_destructor (notmuch_message_file_t
*message)
/* Create a new notmuch_message_file_t for 'filename' with 'ctx' as
* the talloc owner. */
notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename)
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+ void *ctx, const char *filename)
{
notmuch_message_file_t *message;
@@ -98,16 +99,18 @@ _notmuch_message_file_open_ctx (void *ctx, const char
*filename)
return message;
FAIL:
-fprintf (stderr, Error opening %s: %s\n, filename, strerror (errno));
+_notmuch_database_log (notmuch, Error opening %s: %s\n,
+ filename, strerror (errno));
_notmuch_message_file_close (message);
return NULL;
}
notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename)
+_notmuch_message_file_open (notmuch_database_t *notmuch,
+ const char *filename)
{
-return _notmuch_message_file_open_ctx (NULL, filename);
+return _notmuch_message_file_open_ctx (notmuch, NULL, filename);
}
void
diff --git a/lib/message.cc b/lib/message.cc
index 541eabc..4251b44 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -437,7 +437,8 @@ _notmuch_message_ensure_message_file (notmuch_message_t
*message)
if (unlikely (filename == NULL))
return;
-message-message_file = _notmuch_message_file_open_ctx (message, filename);
+message-message_file = _notmuch_message_file_open_ctx (
+ _notmuch_message_database (message), message, filename);
}
const char *
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index fad3a92..76a2f33 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -359,11 +359,12 @@ typedef struct _notmuch_message_file
notmuch_message_file_t;
* Returns NULL if any error occurs.
*/
notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename);
+_notmuch_message_file_open (notmuch_database_t *notmuch, const char *filename);
/* Like notmuch_message_file_open but with 'ctx' as the talloc owner. */
notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename);
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+ void *ctx, const char *filename);
/* Close a notmuch message previously opened with notmuch_message_open. */
void
--
2.1.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v3 1/6] lib: add verbose versions of notmuch_database_{open,create}
This has the the non-trivial side effect silencing all output to
stderror from these two routines.
The stdargs based infrastucture will be used in following commits for
a more general logging mechanism.
The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass. The fact that no other changes are needed is
probably a sign of weakness in the test suite.
---
lib/database.cc | 88 -
lib/notmuch.h | 21 +
notmuch-new.c | 8 +++--
notmuch-search.c| 8 +++--
test/symbol-test.cc | 6 +++-
5 files changed, 111 insertions(+), 20 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 3601f9d..4f4e871 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -343,6 +343,35 @@ notmuch_status_to_string (notmuch_status_t status)
}
static void
+vlog_to_string (void *ctx,
+ char **status_string,
+ const char *format,
+ va_list va_args)
+{
+if (!status_string)
+ return;
+
+if (*status_string)
+ talloc_free (*status_string);
+
+*status_string = talloc_vasprintf (ctx, format, va_args);
+}
+
+static void
+log_to_string (char **str,
+ const char *format,
+ ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vlog_to_string (NULL, str, format, va_args);
+
+va_end (va_args);
+}
+
+static void
find_doc_ids_for_term (notmuch_database_t *notmuch,
const char *term,
Xapian::PostingIterator *begin,
@@ -602,28 +631,37 @@ parse_references (void *ctx,
notmuch_status_t
notmuch_database_create (const char *path, notmuch_database_t **database)
{
+return notmuch_database_create_verbose (path, database, NULL);
+}
+
+notmuch_status_t
+notmuch_database_create_verbose (const char *path,
+notmuch_database_t **database,
+char **status_string)
+{
notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
notmuch_database_t *notmuch = NULL;
char *notmuch_path = NULL;
+char *message = NULL;
struct stat st;
int err;
if (path == NULL) {
- fprintf (stderr, Error: Cannot create a database for a NULL path.\n);
+ log_to_string (message, Error: Cannot create a database for a NULL
path.\n);
status = NOTMUCH_STATUS_NULL_POINTER;
goto DONE;
}
err = stat (path, st);
if (err) {
- fprintf (stderr, Error: Cannot create database at %s: %s.\n,
+ log_to_string (message, Error: Cannot create database at %s: %s.\n,
path, strerror (errno));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
if (! S_ISDIR (st.st_mode)) {
- fprintf (stderr, Error: Cannot create database at %s: Not a
directory.\n,
+ log_to_string (message, Error: Cannot create database at %s: Not a
directory.\n,
path);
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -634,15 +672,15 @@ notmuch_database_create (const char *path,
notmuch_database_t **database)
err = mkdir (notmuch_path, 0755);
if (err) {
- fprintf (stderr, Error: Cannot create directory %s: %s.\n,
+ log_to_string (message, Error: Cannot create directory %s: %s.\n,
notmuch_path, strerror (errno));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
-status = notmuch_database_open (path,
- NOTMUCH_DATABASE_MODE_READ_WRITE,
- notmuch);
+status = notmuch_database_open_verbose (path,
+ NOTMUCH_DATABASE_MODE_READ_WRITE,
+ notmuch, message);
if (status)
goto DONE;
@@ -660,6 +698,8 @@ notmuch_database_create (const char *path,
notmuch_database_t **database)
if (notmuch_path)
talloc_free (notmuch_path);
+if (message)
+ *status_string = strdup(message);
if (database)
*database = notmuch;
else
@@ -760,37 +800,47 @@ notmuch_database_open (const char *path,
notmuch_database_mode_t mode,
notmuch_database_t **database)
{
+return notmuch_database_open_verbose(path, mode, database, NULL);
+}
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+ notmuch_database_mode_t mode,
+ notmuch_database_t **database,
+ char **status_string)
+{
notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
void *local = talloc_new (NULL);
notmuch_database_t *notmuch = NULL;
char *notmuch_path, *xapian_path, *incompat_features;
+char *message = NULL;
struct stat st;
int err;
unsigned int i, version;
static int initialized = 0;
if (path == NULL) {
- fprintf (stderr,
[Patch v3 3/6] lib: add a log function with output to a string in notmuch_database_t
This is a thin wrapper around the previously implemented string
logging infrastructure. In the future it could have more configurable
output options.
---
lib/database.cc | 14 ++
lib/notmuch-private.h | 4
2 files changed, 18 insertions(+)
diff --git a/lib/database.cc b/lib/database.cc
index 18db902..9af1a47 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -371,6 +371,20 @@ log_to_string (char **str,
va_end (va_args);
}
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+ const char *format,
+ ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vlog_to_string (notmuch, notmuch-status_string, format, va_args);
+
+va_end (va_args);
+}
+
static void
find_doc_ids_for_term (notmuch_database_t *notmuch,
const char *term,
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 012ad25..7c6cfc0 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -191,6 +191,10 @@ _notmuch_message_id_compressed (void *ctx, const char
*message_id);
notmuch_status_t
_notmuch_database_ensure_writable (notmuch_database_t *notmuch);
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+ const char *format, ...);
+
const char *
_notmuch_database_relative_path (notmuch_database_t *notmuch,
const char *path);
--
2.1.3
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v3 5/6] lib: replace almost all fprintfs in library with _n_d_log
This is not supposed to change any functionality from an end user
point of view. Note that it will eliminate some output to stderr. The
query debugging output is left as is; it doesn't really fit with the
current primitive logging model. The remaining bad fprintf will need
an internal API change.
---
lib/database.cc | 34 +-
lib/directory.cc | 4 ++--
lib/index.cc | 11 +++
lib/message.cc | 6 +++---
lib/query.cc | 8
5 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 9af1a47..19f5f0e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -493,7 +493,7 @@ notmuch_database_find_message (notmuch_database_t *notmuch,
return NOTMUCH_STATUS_SUCCESS;
} catch (const Xapian::Error error) {
- fprintf (stderr, A Xapian exception occurred finding message: %s.\n,
+ _notmuch_database_log (notmuch, A Xapian exception occurred finding
message: %s.\n,
error.get_msg().c_str());
notmuch-exception_reported = TRUE;
*message_ret = NULL;
@@ -725,7 +725,7 @@ notmuch_status_t
_notmuch_database_ensure_writable (notmuch_database_t *notmuch)
{
if (notmuch-mode == NOTMUCH_DATABASE_MODE_READ_ONLY) {
- fprintf (stderr, Cannot write to a read-only database.\n);
+ _notmuch_database_log (notmuch, Cannot write to a read-only
database.\n);
return NOTMUCH_STATUS_READ_ONLY_DATABASE;
}
@@ -1010,7 +1010,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
} catch (const Xapian::Error error) {
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
if (! notmuch-exception_reported) {
- fprintf (stderr, Error: A Xapian exception occurred closing
database: %s\n,
+ _notmuch_database_log (notmuch, Error: A Xapian exception
occurred closing database: %s\n,
error.get_msg().c_str());
}
}
@@ -1142,12 +1142,12 @@ notmuch_database_compact (const char *path,
}
if (stat (backup_path, statbuf) != -1) {
- fprintf (stderr, Path already exists: %s\n, backup_path);
+ _notmuch_database_log (notmuch, Path already exists: %s\n,
backup_path);
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
if (errno != ENOENT) {
- fprintf (stderr, Unknown error while stat()ing path: %s\n,
+ _notmuch_database_log (notmuch, Unknown error while stat()ing path:
%s\n,
strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1167,20 +1167,20 @@ notmuch_database_compact (const char *path,
compactor.set_destdir (compact_xapian_path);
compactor.compact ();
} catch (const Xapian::Error error) {
- fprintf (stderr, Error while compacting: %s\n,
error.get_msg().c_str());
+ _notmuch_database_log (notmuch, Error while compacting: %s\n,
error.get_msg().c_str());
ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
goto DONE;
}
if (rename (xapian_path, backup_path)) {
- fprintf (stderr, Error moving %s to %s: %s\n,
+ _notmuch_database_log (notmuch, Error moving %s to %s: %s\n,
xapian_path, backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
if (rename (compact_xapian_path, xapian_path)) {
- fprintf (stderr, Error moving %s to %s: %s\n,
+ _notmuch_database_log (notmuch, Error moving %s to %s: %s\n,
compact_xapian_path, xapian_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1188,7 +1188,7 @@ notmuch_database_compact (const char *path,
if (! keep_backup) {
if (rmtree (backup_path)) {
- fprintf (stderr, Error removing old database %s: %s\n,
+ _notmuch_database_log (notmuch, Error removing old database %s:
%s\n,
backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1217,7 +1217,7 @@ notmuch_database_compact (unused (const char *path),
unused (notmuch_compact_status_cb_t status_cb),
unused (void *closure))
{
-fprintf (stderr, notmuch was compiled against a xapian version lacking
compaction support.\n);
+_notmuch_database_log (notmuch, notmuch was compiled against a xapian
version lacking compaction support.\n);
return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
}
#endif
@@ -1495,7 +1495,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
}
if (private_status) {
- fprintf (stderr,
+ _notmuch_database_log (notmuch,
Upgrade failed while creating ghost messages.\n);
status = COERCE_STATUS (private_status, Unexpected status from
_notmuch_message_initialize_ghost);
goto DONE;
@@ -1545,7 +1545,7 @@
Re: Solaris 11 anyone?
Thanks so much for your response, David. I've used notmuch for years, and really appreciate it (and rely on it!). The switch to solaris has deprived me... the problems are purely around the build dependencies. All the best Jamie On Wed, Dec 24, 2014 at 01:28:14PM -0700, David Bremner-2 [via notmuch] wrote: jsdckr jsd...@me.com writes: Greetings I'm having such difficulties compiling notmuch on Solaris 11 x86 - been working with the patched code, but no success so far. Does anyone, by any chance, have a binary available? Sorry I can't help with Solaris binaries. Out of curiousity are the problems you are having with getting the build-dependencies for notmuch compiled, or with notmuch itself? d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch ___ If you reply to this email, your message will be added to the discussion below: http://notmuch.198994.n3.nabble.com/Solaris-11-anyone-tp4032762p4032764.html To unsubscribe from Solaris 11 anyone?, visit http://notmuch.198994.n3.nabble.com/template/NamlServlet.jtp?macro=unsubscribe_by_codenode=4032762code=anNkY2tyQG1lLmNvbXw0MDMyNzYyfDEzMzIwOTkzNjA= -- View this message in context: http://notmuch.198994.n3.nabble.com/Solaris-11-anyone-tp4032762p4032790.html Sent from the notmuch mailing list archive at Nabble.com. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] lib: another iterator-temporary/stale-pointer bug
Tamas Szakaly points out [1] that the bug fixed in 51b073c still exists in at least one place. This change follows the suggestion of [2] and creates a block scope temporary std::string to avoid the rules of iterators temporaries. [1]: id:20141226113755.GA64154 at pamparam [2]: id:20141226230655.GA41992 at pamparam --- I decided to take a more minimalist approach than [2]. In particular using "direntry" for two different things seemed slightly trickier than necessary, for no obvious performance gain (calling .c_str() should be cheap). lib/message.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/message.cc b/lib/message.cc index a7a13cc..bacb4d4 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -641,15 +641,16 @@ _notmuch_message_add_directory_terms (void *ctx, notmuch_message_t *message) unsigned int directory_id; const char *direntry, *directory; char *colon; + const std::string term = *i; /* Terminate loop at first term without desired prefix. */ - if (strncmp ((*i).c_str (), direntry_prefix, direntry_prefix_len)) + if (strncmp (term.c_str (), direntry_prefix, direntry_prefix_len)) break; /* Indicate that there are filenames remaining. */ status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID; - direntry = (*i).c_str (); + direntry = term.c_str (); direntry += direntry_prefix_len; directory_id = strtol (direntry, , 10); -- 2.1.3
libnotmuch logging overhaul v3
changes since the last round [1]
- remove unused variable
- fix the internal usage of n_d_open to use _verbose version
- revert the hiding of query debugging.
[1]: id:1419717937-1108-2-git-send-email-david at tethera.net
diff --git a/lib/database.cc b/lib/database.cc
index 16cd940..b144885 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -377,7 +377,6 @@ _notmuch_database_log (notmuch_database_t *notmuch,
...)
{
va_list va_args;
-const char *message;
va_start (va_args, format);
@@ -693,9 +692,9 @@ notmuch_database_create_verbose (const char *path,
goto DONE;
}
-status = notmuch_database_open (path,
- NOTMUCH_DATABASE_MODE_READ_WRITE,
- );
+status = notmuch_database_open_verbose (path,
+ NOTMUCH_DATABASE_MODE_READ_WRITE,
+ , );
if (status)
goto DONE;
@@ -1101,13 +1100,18 @@ notmuch_database_compact (const char *path,
notmuch_database_t *notmuch = NULL;
struct stat statbuf;
notmuch_bool_t keep_backup;
+char *message = NULL;
local = talloc_new (NULL);
if (! local)
return NOTMUCH_STATUS_OUT_OF_MEMORY;
-ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE,
);
+ret = notmuch_database_open_verbose (path,
+NOTMUCH_DATABASE_MODE_READ_WRITE,
+,
+);
if (ret) {
+ if (status_cb) status_cb (message, closure);
goto DONE;
}
diff --git a/lib/query.cc b/lib/query.cc
index 2e20ab2..9ed420c 100644
--- a/lib/query.cc
+++ b/lib/query.cc
@@ -78,7 +78,7 @@ notmuch_query_create (notmuch_database_t *notmuch,
notmuch_query_t *query;
if (_debug_query ())
- _notmuch_database_log (notmuch, "Query string is:\n%s\n", query_string);
+ fprintf (stderr, "Query string is:\n%s\n", query_string);
query = talloc (notmuch, notmuch_query_t);
if (unlikely (query == NULL))
@@ -266,9 +266,9 @@ notmuch_query_search_messages (notmuch_query_t *query)
}
if (_debug_query ()) {
- _notmuch_database_log (notmuch, "Exclude query is:\n%s\n",
+ fprintf (stderr, "Exclude query is:\n%s\n",
exclude_query.get_description ().c_str ());
- _notmuch_database_log (notmuch, "Final query is:\n%s\n",
+ fprintf (stderr, "Final query is:\n%s\n",
final_query.get_description ().c_str ());
}
@@ -549,9 +549,9 @@ notmuch_query_count_messages (notmuch_query_t *query)
enquire.set_docid_order(Xapian::Enquire::ASCENDING);
if (_debug_query ()) {
- _notmuch_database_log (notmuch, "Exclude query is:\n%s\n",
+ fprintf (stderr, "Exclude query is:\n%s\n",
exclude_query.get_description ().c_str ());
- _notmuch_database_log (notmuch, "Final query is:\n%s\n",
+ fprintf (stderr, "Final query is:\n%s\n",
final_query.get_description ().c_str ());
}
lib/database.cc | 14 +-
lib/query.cc| 10 +-
2 files changed, 14 insertions(+), 10 deletions(-)
[Patch v3 4/6] lib: add private function to extract the database for a message.
This is needed by logging in functions outside message.cc that take
only a notmuch_message_t object.
---
lib/message.cc| 6 ++
lib/notmuch-private.h | 2 ++
2 files changed, 8 insertions(+)
diff --git a/lib/message.cc b/lib/message.cc
index a7a13cc..43cc078 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1625,3 +1625,9 @@ notmuch_message_destroy (notmuch_message_t *message)
{
talloc_free (message);
}
+
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message)
+{
+return message->notmuch;
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 7c6cfc0..fad3a92 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -473,6 +473,8 @@ _notmuch_doc_id_set_remove (notmuch_doc_id_set_t *doc_ids,
void
_notmuch_message_add_reply (notmuch_message_t *message,
notmuch_message_t *reply);
+notmuch_database_t *
+_notmuch_message_database (notmuch_message_t *message);
/* sha1.c */
--
2.1.3
[Patch v3 6/6] lib: eliminate fprintf from _notmuch_message_file_open
You may wonder why _notmuch_message_file_open_ctx has two parameters.
This is because we need sometime to use a ctx which is a
notmuch_message_t. While we could get the database from this, there is
no easy way in C to tell type we are getting. The remaining fprintf is
removed by Jani's series un-deprecating single message mboxes.
---
lib/database.cc | 2 +-
lib/message-file.c| 11 +++
lib/message.cc| 3 ++-
lib/notmuch-private.h | 5 +++--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 19f5f0e..b144885 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -2298,7 +2298,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
if (ret)
return ret;
-message_file = _notmuch_message_file_open (filename);
+message_file = _notmuch_message_file_open (notmuch, filename);
if (message_file == NULL)
return NOTMUCH_STATUS_FILE_ERROR;
diff --git a/lib/message-file.c b/lib/message-file.c
index eda1b74..1f55cdf 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -76,7 +76,8 @@ _notmuch_message_file_destructor (notmuch_message_file_t
*message)
/* Create a new notmuch_message_file_t for 'filename' with 'ctx' as
* the talloc owner. */
notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename)
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+ void *ctx, const char *filename)
{
notmuch_message_file_t *message;
@@ -98,16 +99,18 @@ _notmuch_message_file_open_ctx (void *ctx, const char
*filename)
return message;
FAIL:
-fprintf (stderr, "Error opening %s: %s\n", filename, strerror (errno));
+_notmuch_database_log (notmuch, "Error opening %s: %s\n",
+ filename, strerror (errno));
_notmuch_message_file_close (message);
return NULL;
}
notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename)
+_notmuch_message_file_open (notmuch_database_t *notmuch,
+ const char *filename)
{
-return _notmuch_message_file_open_ctx (NULL, filename);
+return _notmuch_message_file_open_ctx (notmuch, NULL, filename);
}
void
diff --git a/lib/message.cc b/lib/message.cc
index 541eabc..4251b44 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -437,7 +437,8 @@ _notmuch_message_ensure_message_file (notmuch_message_t
*message)
if (unlikely (filename == NULL))
return;
-message->message_file = _notmuch_message_file_open_ctx (message, filename);
+message->message_file = _notmuch_message_file_open_ctx (
+ _notmuch_message_database (message), message, filename);
}
const char *
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index fad3a92..76a2f33 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -359,11 +359,12 @@ typedef struct _notmuch_message_file
notmuch_message_file_t;
* Returns NULL if any error occurs.
*/
notmuch_message_file_t *
-_notmuch_message_file_open (const char *filename);
+_notmuch_message_file_open (notmuch_database_t *notmuch, const char *filename);
/* Like notmuch_message_file_open but with 'ctx' as the talloc owner. */
notmuch_message_file_t *
-_notmuch_message_file_open_ctx (void *ctx, const char *filename);
+_notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
+ void *ctx, const char *filename);
/* Close a notmuch message previously opened with notmuch_message_open. */
void
--
2.1.3
[Patch v3 2/6] lib/database: add field for last error string
The idea is to have a logging function setting this string instead of
printing to stderr.
---
lib/database-private.h | 4
lib/database.cc| 7 +++
lib/notmuch.h | 7 +++
3 files changed, 18 insertions(+)
diff --git a/lib/database-private.h b/lib/database-private.h
index 15e03cc..7efd98b 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -146,6 +146,10 @@ struct _notmuch_database {
unsigned int last_doc_id;
uint64_t last_thread_id;
+/* error reporting; this value persists only until the
+ * next library call. May be NULL */
+char *status_string;
+
Xapian::QueryParser *query_parser;
Xapian::TermGenerator *term_gen;
Xapian::ValueRangeProcessor *value_range_processor;
diff --git a/lib/database.cc b/lib/database.cc
index 4f4e871..18db902 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -858,6 +858,7 @@ notmuch_database_open_verbose (const char *path,
notmuch = talloc_zero (NULL, notmuch_database_t);
notmuch->exception_reported = FALSE;
+notmuch->status_string = NULL;
notmuch->path = talloc_strdup (notmuch, path);
if (notmuch->path[strlen (notmuch->path) - 1] == '/')
@@ -2543,3 +2544,9 @@ notmuch_database_get_all_tags (notmuch_database_t *db)
return NULL;
}
}
+
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch)
+{
+return notmuch->status_string;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 0dfac8f..2ab2998 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -302,6 +302,13 @@ notmuch_database_open_verbose (const char *path,
char **error_message);
/**
+ * Retrieve last status string for given database.
+ *
+ */
+const char *
+notmuch_database_status_string (notmuch_database_t *notmuch);
+
+/**
* Commit changes and close the given notmuch database.
*
* After notmuch_database_close has been called, calls to other
--
2.1.3
[Patch v3 3/6] lib: add a log function with output to a string in notmuch_database_t
This is a thin wrapper around the previously implemented string
logging infrastructure. In the future it could have more configurable
output options.
---
lib/database.cc | 14 ++
lib/notmuch-private.h | 4
2 files changed, 18 insertions(+)
diff --git a/lib/database.cc b/lib/database.cc
index 18db902..9af1a47 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -371,6 +371,20 @@ log_to_string (char **str,
va_end (va_args);
}
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+ const char *format,
+ ...)
+{
+va_list va_args;
+
+va_start (va_args, format);
+
+vlog_to_string (notmuch, >status_string, format, va_args);
+
+va_end (va_args);
+}
+
static void
find_doc_ids_for_term (notmuch_database_t *notmuch,
const char *term,
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 012ad25..7c6cfc0 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -191,6 +191,10 @@ _notmuch_message_id_compressed (void *ctx, const char
*message_id);
notmuch_status_t
_notmuch_database_ensure_writable (notmuch_database_t *notmuch);
+void
+_notmuch_database_log (notmuch_database_t *notmuch,
+ const char *format, ...);
+
const char *
_notmuch_database_relative_path (notmuch_database_t *notmuch,
const char *path);
--
2.1.3
[Patch v3 5/6] lib: replace almost all fprintfs in library with _n_d_log
This is not supposed to change any functionality from an end user
point of view. Note that it will eliminate some output to stderr. The
query debugging output is left as is; it doesn't really fit with the
current primitive logging model. The remaining "bad" fprintf will need
an internal API change.
---
lib/database.cc | 34 +-
lib/directory.cc | 4 ++--
lib/index.cc | 11 +++
lib/message.cc | 6 +++---
lib/query.cc | 8
5 files changed, 33 insertions(+), 30 deletions(-)
diff --git a/lib/database.cc b/lib/database.cc
index 9af1a47..19f5f0e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -493,7 +493,7 @@ notmuch_database_find_message (notmuch_database_t *notmuch,
return NOTMUCH_STATUS_SUCCESS;
} catch (const Xapian::Error ) {
- fprintf (stderr, "A Xapian exception occurred finding message: %s.\n",
+ _notmuch_database_log (notmuch, "A Xapian exception occurred finding
message: %s.\n",
error.get_msg().c_str());
notmuch->exception_reported = TRUE;
*message_ret = NULL;
@@ -725,7 +725,7 @@ notmuch_status_t
_notmuch_database_ensure_writable (notmuch_database_t *notmuch)
{
if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY) {
- fprintf (stderr, "Cannot write to a read-only database.\n");
+ _notmuch_database_log (notmuch, "Cannot write to a read-only
database.\n");
return NOTMUCH_STATUS_READ_ONLY_DATABASE;
}
@@ -1010,7 +1010,7 @@ notmuch_database_close (notmuch_database_t *notmuch)
} catch (const Xapian::Error ) {
status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
if (! notmuch->exception_reported) {
- fprintf (stderr, "Error: A Xapian exception occurred closing
database: %s\n",
+ _notmuch_database_log (notmuch, "Error: A Xapian exception
occurred closing database: %s\n",
error.get_msg().c_str());
}
}
@@ -1142,12 +1142,12 @@ notmuch_database_compact (const char *path,
}
if (stat (backup_path, ) != -1) {
- fprintf (stderr, "Path already exists: %s\n", backup_path);
+ _notmuch_database_log (notmuch, "Path already exists: %s\n",
backup_path);
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
if (errno != ENOENT) {
- fprintf (stderr, "Unknown error while stat()ing path: %s\n",
+ _notmuch_database_log (notmuch, "Unknown error while stat()ing path:
%s\n",
strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1167,20 +1167,20 @@ notmuch_database_compact (const char *path,
compactor.set_destdir (compact_xapian_path);
compactor.compact ();
} catch (const Xapian::Error ) {
- fprintf (stderr, "Error while compacting: %s\n",
error.get_msg().c_str());
+ _notmuch_database_log (notmuch, "Error while compacting: %s\n",
error.get_msg().c_str());
ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
goto DONE;
}
if (rename (xapian_path, backup_path)) {
- fprintf (stderr, "Error moving %s to %s: %s\n",
+ _notmuch_database_log (notmuch, "Error moving %s to %s: %s\n",
xapian_path, backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
if (rename (compact_xapian_path, xapian_path)) {
- fprintf (stderr, "Error moving %s to %s: %s\n",
+ _notmuch_database_log (notmuch, "Error moving %s to %s: %s\n",
compact_xapian_path, xapian_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1188,7 +1188,7 @@ notmuch_database_compact (const char *path,
if (! keep_backup) {
if (rmtree (backup_path)) {
- fprintf (stderr, "Error removing old database %s: %s\n",
+ _notmuch_database_log (notmuch, "Error removing old database %s:
%s\n",
backup_path, strerror (errno));
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
@@ -1217,7 +1217,7 @@ notmuch_database_compact (unused (const char *path),
unused (notmuch_compact_status_cb_t status_cb),
unused (void *closure))
{
-fprintf (stderr, "notmuch was compiled against a xapian version lacking
compaction support.\n");
+_notmuch_database_log (notmuch, "notmuch was compiled against a xapian
version lacking compaction support.\n");
return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
}
#endif
@@ -1495,7 +1495,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
}
if (private_status) {
- fprintf (stderr,
+ _notmuch_database_log (notmuch,
"Upgrade failed while creating ghost messages.\n");
status = COERCE_STATUS (private_status, "Unexpected status from
_notmuch_message_initialize_ghost");
goto DONE;
@@ -1545,7 +1545,7 @@
