* libnotmuch logging overhaul v4
@ 2015-03-14 17:02 David Bremner
2015-03-14 17:02 ` [Patch v4 1/9] test: Add two tests for error output from notmuch_database_open David Bremner
` (8 more replies)
0 siblings, 9 replies; 34+ messages in thread
From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw)
To: notmuch
Obsoletes
id:1419788030-10567-1-git-send-email-david@tethera.net
The bad news is it is 3 patches longer; the good news is that I think
those patches are just testing and make sense whether we do the rest
or not. The first 4 patches together now have the have the effect
adding a "quiet" version of open and create, slightly perversely named
"_verbose". As always, naming things is hard.
Patches 5 through 7 are just support for patch 8,
and are unchanged from the previous version.
[Patch v4 5/9] lib/database: add field for last error string
[Patch v4 6/9] lib: add a log function with output to a string in
[Patch v4 7/9] lib: add private function to extract the database for
I'm not sure about the way notmuch_database_compact is handled here;
maybe it should use an extra string argument like
notmuch_database_open.
[Patch v4 8/9] lib: replace almost all fprintfs in library with
Unmodified from the previous series.
[Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open
^ permalink raw reply [flat|nested] 34+ messages in thread* [Patch v4 1/9] test: Add two tests for error output from notmuch_database_open 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-14 17:02 ` [Patch v4 2/9] test: add support for compiling and running C snippets David Bremner ` (7 subsequent siblings) 8 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch This is arguably testing the same thing twice, but in the brave new future where we don't use printf anymore, each subcommand will be responsible for handling the output on it's own. --- test/T050-new.sh | 7 +++++++ test/T150-tagging.sh | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/test/T050-new.sh b/test/T050-new.sh index 7119356..e6c3291 100755 --- a/test/T050-new.sh +++ b/test/T050-new.sh @@ -276,4 +276,11 @@ test_expect_code 1 "Invalid tags set exit code" \ notmuch config set new.tags $OLDCONFIG + +test_begin_subtest "Xapian exception: read only files" +chmod u-w ${MAIL_DIR}/.notmuch/xapian/*.DB +output=$(NOTMUCH_NEW 2>&1 | sed 's/: .*$//' ) +chmod u+w ${MAIL_DIR}/.notmuch/xapian/*.DB +test_expect_equal "$output" "A Xapian exception occurred opening database" + test_done diff --git a/test/T150-tagging.sh b/test/T150-tagging.sh index 45471ac..4a2673d 100755 --- a/test/T150-tagging.sh +++ b/test/T150-tagging.sh @@ -261,4 +261,10 @@ test_expect_code 1 "Empty tag names" 'notmuch tag + One' test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One' +test_begin_subtest "Xapian exception: read only files" +chmod u-w ${MAIL_DIR}/.notmuch/xapian/*.DB +output=$(notmuch tag +something '*' 2>&1 | sed 's/: .*$//' ) +chmod u+w ${MAIL_DIR}/.notmuch/xapian/*.DB +test_expect_equal "$output" "A Xapian exception occurred opening database" + test_done -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v4 2/9] test: add support for compiling and running C snippets 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner 2015-03-14 17:02 ` [Patch v4 1/9] test: Add two tests for error output from notmuch_database_open David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-21 8:51 ` Tomi Ollila 2015-03-14 17:02 ` [Patch v4 3/9] test: add error reporting tests for lib/database.cc David Bremner ` (6 subsequent siblings) 8 siblings, 1 reply; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch This is to limit the copy-pasta involved in running C tests. I decided to keep things simple and not try to provide an actual C skeleton. The setting of LD_LIBRARY_PATH is to force using the built libnotmuch rather than any potential system one. --- test/README | 5 +++++ test/test-lib.sh | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/test/README b/test/README index 81a1c82..5b40474 100644 --- a/test/README +++ b/test/README @@ -84,6 +84,11 @@ the tests in one of the following ways. TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient +Some tests may require a c compiler. You can choose the name and flags similarly +to with emacs, e.g. + + make test TEST_CC=gcc TEST_CFLAGS="-g -O2" + Quiet Execution --------------- diff --git a/test/test-lib.sh b/test/test-lib.sh index 133fbe4..c7af003 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -73,6 +73,8 @@ if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \ fi TEST_EMACS=${TEST_EMACS:-${EMACS:-emacs}} TEST_EMACSCLIENT=${TEST_EMACSCLIENT:-emacsclient} +TEST_CC=${TEST_CC:-cc} +TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"} # Protect ourselves from common misconfiguration to export # CDPATH into the environment @@ -1161,6 +1163,19 @@ test_python() { | $cmd - } +test_C() { + test_file="test${test_count}.c" + exec_file=${test_file%%.c} + cat > ${test_file} + export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib + ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch + echo "== stdout ==" > OUTPUT.stdout + echo "== stderr ==" > OUTPUT.stderr + ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr + cat OUTPUT.stdout OUTPUT.stderr | sed "s,$(pwd),CWD," > OUTPUT +} + + # Creates a script that counts how much time it is executed and calls # notmuch. $notmuch_counter_command is set to the path to the # generated script. Use notmuch_counter_value() function to get the -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Patch v4 2/9] test: add support for compiling and running C snippets 2015-03-14 17:02 ` [Patch v4 2/9] test: add support for compiling and running C snippets David Bremner @ 2015-03-21 8:51 ` Tomi Ollila 0 siblings, 0 replies; 34+ messages in thread From: Tomi Ollila @ 2015-03-21 8:51 UTC (permalink / raw) To: David Bremner, notmuch On Sat, Mar 14 2015, David Bremner <david@tethera.net> wrote: > This is to limit the copy-pasta involved in running C tests. I decided Wat kind of spaghetti have you been eating... ;D > to keep things simple and not try to provide an actual C skeleton. > > The setting of LD_LIBRARY_PATH is to force using the built libnotmuch > rather than any potential system one. > --- > test/README | 5 +++++ > test/test-lib.sh | 15 +++++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/test/README b/test/README > index 81a1c82..5b40474 100644 > --- a/test/README > +++ b/test/README > @@ -84,6 +84,11 @@ the tests in one of the following ways. > TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs > make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient > > +Some tests may require a c compiler. You can choose the name and flags similarly > +to with emacs, e.g. > + > + make test TEST_CC=gcc TEST_CFLAGS="-g -O2" > + > Quiet Execution > --------------- > > diff --git a/test/test-lib.sh b/test/test-lib.sh > index 133fbe4..c7af003 100644 > --- a/test/test-lib.sh > +++ b/test/test-lib.sh > @@ -73,6 +73,8 @@ if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \ > fi > TEST_EMACS=${TEST_EMACS:-${EMACS:-emacs}} > TEST_EMACSCLIENT=${TEST_EMACSCLIENT:-emacsclient} > +TEST_CC=${TEST_CC:-cc} > +TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"} > > # Protect ourselves from common misconfiguration to export > # CDPATH into the environment > @@ -1161,6 +1163,19 @@ test_python() { > | $cmd - > } > > +test_C() { for consistency test_C () { > + test_file="test${test_count}.c" > + exec_file=${test_file%%.c} These could be other way around, if not, exec_file=${test_file%.c} (i.e. only one % is enough). > + cat > ${test_file} > + export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib > + ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch > + echo "== stdout ==" > OUTPUT.stdout > + echo "== stderr ==" > OUTPUT.stderr > + ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr > + cat OUTPUT.stdout OUTPUT.stderr | sed "s,$(pwd),CWD," > OUTPUT sed "s,${PWD},CWD," OUTPUT.stdout OUTPUT.stderr > OUTPUT ^ permalink raw reply [flat|nested] 34+ messages in thread
* [Patch v4 3/9] test: add error reporting tests for lib/database.cc 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner 2015-03-14 17:02 ` [Patch v4 1/9] test: Add two tests for error output from notmuch_database_open David Bremner 2015-03-14 17:02 ` [Patch v4 2/9] test: add support for compiling and running C snippets David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-14 17:02 ` [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner ` (5 subsequent siblings) 8 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch Unfortunately quite a few of the error handling paths here require more sophisticated tests using e.g. gdb. --- test/T560-lib-error.sh | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100755 test/T560-lib-error.sh diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh new file mode 100755 index 0000000..dc2022f --- /dev/null +++ b/test/T560-lib-error.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +test_description="error reporting for library" + +. ./test-lib.sh + +add_email_corpus + +test_expect_success "building database" "NOTMUCH_NEW" + +test_begin_subtest "Open null pointer" +test_C <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (NULL, 0, 0); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error: Cannot open a database for a NULL path. +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Open nonexistent database" +test_C <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open ("/nonexistent/foo", 0, 0); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error opening database at /nonexistent/foo/.notmuch: No such file or directory +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Write to read-only database" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_ONLY, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + stat = notmuch_database_add_message (db, "/dev/null", NULL); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Cannot write to a read-only database. +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "compact, overwriting existing backup" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +static void +status_cb (const char *msg, void *closure) +{ + printf ("%s\n", msg); +} +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_compact (argv[1], argv[1], status_cb, NULL); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Path already exists: CWD/mail +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_done -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open,create} 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner ` (2 preceding siblings ...) 2015-03-14 17:02 ` [Patch v4 3/9] test: add error reporting tests for lib/database.cc David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-21 9:27 ` [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open, create} Tomi Ollila 2015-03-14 17:02 ` [Patch v4 5/9] lib/database: add field for last error string David Bremner ` (4 subsequent siblings) 8 siblings, 1 reply; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch The compatibility wrapper ensures that clients calling notmuch_database_open will receive consistent output for now. 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. --- lib/database.cc | 96 ++++++++++++++++++++++++++++++++++++++++++++--------- lib/notmuch.h | 21 ++++++++++++ notmuch-new.c | 8 +++-- notmuch-search.c | 8 +++-- test/symbol-test.cc | 6 +++- 5 files changed, 119 insertions(+), 20 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 3974e2e..b774edc 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -349,6 +349,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, @@ -608,28 +637,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; @@ -640,15 +678,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, - ¬much); + status = notmuch_database_open_verbose (path, + NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, &message); if (status) goto DONE; @@ -667,6 +705,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 @@ -767,37 +807,55 @@ notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database) { + char *status_string = NULL; + notmuch_status_t status; + + status = notmuch_database_open_verbose(path, mode, database, + &status_string); + + if (status_string) fputs(status_string, stderr); + + return status; +} + +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, "Error: Cannot open a database for a NULL path.\n"); + log_to_string (&message, "Error: Cannot open a database for a NULL path.\n"); status = NOTMUCH_STATUS_NULL_POINTER; goto DONE; } if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) { - fprintf (stderr, "Out of memory\n"); + log_to_string (&message, "Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } err = stat (notmuch_path, &st); if (err) { - fprintf (stderr, "Error opening database at %s: %s\n", + log_to_string (&message, "Error opening database at %s: %s\n", notmuch_path, strerror (errno)); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { - fprintf (stderr, "Out of memory\n"); + log_to_string (&message, "Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } @@ -837,7 +895,7 @@ notmuch_database_open (const char *path, * means a dramatically incompatible change. */ version = notmuch_database_get_version (notmuch); if (version > NOTMUCH_DATABASE_VERSION) { - fprintf (stderr, + log_to_string (&message, "Error: Notmuch database at %s\n" " has a newer database format version (%u) than supported by this\n" " version of notmuch (%u).\n", @@ -856,7 +914,7 @@ notmuch_database_open (const char *path, version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r', &incompat_features); if (incompat_features) { - fprintf (stderr, + log_to_string (&message, "Error: Notmuch database at %s\n" " requires features (%s)\n" " not supported by this version of notmuch.\n", @@ -906,7 +964,7 @@ notmuch_database_open (const char *path, notmuch->query_parser->add_prefix (prefix->name, prefix->prefix); } } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred opening database: %s\n", + log_to_string (&message, "A Xapian exception occurred opening database: %s\n", error.get_msg().c_str()); notmuch_database_destroy (notmuch); notmuch = NULL; @@ -916,6 +974,9 @@ notmuch_database_open (const char *path, DONE: talloc_free (local); + if (status_string && message) + *status_string = strdup (message); + if (database) *database = notmuch; else @@ -1039,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, ¬much); + ret = notmuch_database_open_verbose (path, + NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, + &message); if (ret) { + if (status_cb) status_cb (message, closure); goto DONE; } diff --git a/lib/notmuch.h b/lib/notmuch.h index f066245..c671d82 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -230,6 +230,16 @@ notmuch_status_t notmuch_database_create (const char *path, notmuch_database_t **database); /** + * Like notmuch_database_create, except optionally return an error + * message. This message is allocated by malloc and should be freed by + * the caller. + */ +notmuch_status_t +notmuch_database_create_verbose (const char *path, + notmuch_database_t **database, + char **error_message); + +/** * Database open mode for notmuch_database_open. */ typedef enum { @@ -279,6 +289,17 @@ notmuch_status_t notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database); +/** + * Like notmuch_database_open, except optionally return an error + * message. This message is allocated by malloc and should be freed by + * the caller. + */ + +notmuch_status_t +notmuch_database_open_verbose (const char *path, + notmuch_database_mode_t mode, + notmuch_database_t **database, + char **error_message); /** * Commit changes and close the given notmuch database. diff --git a/notmuch-new.c b/notmuch-new.c index ddf42c1..93b70bf 100644 --- a/notmuch-new.c +++ b/notmuch-new.c @@ -985,9 +985,13 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) return EXIT_FAILURE; add_files_state.total_files = count; } else { - if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, - ¬much)) + char *status_string = NULL; + if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, &status_string)) { + if (status_string) fputs (status_string, stderr); + return EXIT_FAILURE; + } if (notmuch_database_needs_upgrade (notmuch)) { time_t now = time (NULL); diff --git a/notmuch-search.c b/notmuch-search.c index a591d45..d012af3 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar { char *query_str; unsigned int i; + char *status_string = NULL; switch (ctx->format_sel) { case NOTMUCH_FORMAT_TEXT: @@ -570,9 +571,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar notmuch_exit_if_unsupported_format (); - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch)) + if (notmuch_database_open_verbose ( + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) { + if (status_string) fputs (status_string, stderr); return EXIT_FAILURE; + } query_str = query_string_from_args (ctx->notmuch, argc, argv); if (query_str == NULL) { diff --git a/test/symbol-test.cc b/test/symbol-test.cc index 3e96c03..9f8eea7 100644 --- a/test/symbol-test.cc +++ b/test/symbol-test.cc @@ -5,7 +5,11 @@ int main() { notmuch_database_t *notmuch; - notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much); + char *message = NULL; + + if (notmuch_database_open_verbose ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much, &message)) + if (message) fputs (message, stderr); + try { (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN); -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open, create} 2015-03-14 17:02 ` [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner @ 2015-03-21 9:27 ` Tomi Ollila 0 siblings, 0 replies; 34+ messages in thread From: Tomi Ollila @ 2015-03-21 9:27 UTC (permalink / raw) To: David Bremner, notmuch On Sat, Mar 14 2015, David Bremner <david@tethera.net> wrote: > The compatibility wrapper ensures that clients calling > notmuch_database_open will receive consistent output for now. > > The stdargs based infrastucture will be used in following commits for infrastructure :D > a more general logging mechanism. > > The changes to notmuch-{new,search} and test/symbol-test are just to > make the test suite pass. > --- > lib/database.cc | 96 ++++++++++++++++++++++++++++++++++++++++++++--------- > lib/notmuch.h | 21 ++++++++++++ > notmuch-new.c | 8 +++-- > notmuch-search.c | 8 +++-- > test/symbol-test.cc | 6 +++- > 5 files changed, 119 insertions(+), 20 deletions(-) > > diff --git a/lib/database.cc b/lib/database.cc > index 3974e2e..b774edc 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -349,6 +349,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; Noticed just before sending: (! status_string) -- and one more deep down below... > + > + 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); > +} To me it looks a bit peculiar log_to_string () not taking `ctx` as first argument. I'd suggest 1) add that ctx to first arg and tediously add NULL to all callers or 2) rename the function so something less similar and call that instead (even leading _ could "mark" the function to be less generic interface to vlog_to_string) . Otherwise this (and rest of the series) looks pretty good to me. Tomi > +static void > find_doc_ids_for_term (notmuch_database_t *notmuch, > const char *term, > Xapian::PostingIterator *begin, > @@ -608,28 +637,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; > @@ -640,15 +678,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, > - ¬much); > + status = notmuch_database_open_verbose (path, > + NOTMUCH_DATABASE_MODE_READ_WRITE, > + ¬much, &message); > if (status) > goto DONE; > > @@ -667,6 +705,8 @@ notmuch_database_create (const char *path, notmuch_database_t **database) > if (notmuch_path) > talloc_free (notmuch_path); > > + if (message) > + *status_string = strdup(message); strdup (message); > if (database) > *database = notmuch; > else > @@ -767,37 +807,55 @@ notmuch_database_open (const char *path, > notmuch_database_mode_t mode, > notmuch_database_t **database) > { > + char *status_string = NULL; > + notmuch_status_t status; > + > + status = notmuch_database_open_verbose(path, mode, database, > + &status_string); > + > + if (status_string) fputs(status_string, stderr); Also, does this above (and few similar below) match the style elsewhere ? (i personally like it but... `git grep fputs` does not reveal such done before) > + > + return status; > +} > + > +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, "Error: Cannot open a database for a NULL path.\n"); > + log_to_string (&message, "Error: Cannot open a database for a NULL path.\n"); > status = NOTMUCH_STATUS_NULL_POINTER; > goto DONE; > } > > if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) { > - fprintf (stderr, "Out of memory\n"); > + log_to_string (&message, "Out of memory\n"); > status = NOTMUCH_STATUS_OUT_OF_MEMORY; > goto DONE; > } > > err = stat (notmuch_path, &st); > if (err) { > - fprintf (stderr, "Error opening database at %s: %s\n", > + log_to_string (&message, "Error opening database at %s: %s\n", > notmuch_path, strerror (errno)); > status = NOTMUCH_STATUS_FILE_ERROR; > goto DONE; > } > > if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { > - fprintf (stderr, "Out of memory\n"); > + log_to_string (&message, "Out of memory\n"); > status = NOTMUCH_STATUS_OUT_OF_MEMORY; > goto DONE; > } > @@ -837,7 +895,7 @@ notmuch_database_open (const char *path, > * means a dramatically incompatible change. */ > version = notmuch_database_get_version (notmuch); > if (version > NOTMUCH_DATABASE_VERSION) { > - fprintf (stderr, > + log_to_string (&message, > "Error: Notmuch database at %s\n" > " has a newer database format version (%u) than supported by this\n" > " version of notmuch (%u).\n", > @@ -856,7 +914,7 @@ notmuch_database_open (const char *path, > version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r', > &incompat_features); > if (incompat_features) { > - fprintf (stderr, > + log_to_string (&message, > "Error: Notmuch database at %s\n" > " requires features (%s)\n" > " not supported by this version of notmuch.\n", > @@ -906,7 +964,7 @@ notmuch_database_open (const char *path, > notmuch->query_parser->add_prefix (prefix->name, prefix->prefix); > } > } catch (const Xapian::Error &error) { > - fprintf (stderr, "A Xapian exception occurred opening database: %s\n", > + log_to_string (&message, "A Xapian exception occurred opening database: %s\n", > error.get_msg().c_str()); > notmuch_database_destroy (notmuch); > notmuch = NULL; > @@ -916,6 +974,9 @@ notmuch_database_open (const char *path, > DONE: > talloc_free (local); > > + if (status_string && message) > + *status_string = strdup (message); > + > if (database) > *database = notmuch; > else > @@ -1039,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, ¬much); > + ret = notmuch_database_open_verbose (path, > + NOTMUCH_DATABASE_MODE_READ_WRITE, > + ¬much, > + &message); > if (ret) { > + if (status_cb) status_cb (message, closure); > goto DONE; > } > > diff --git a/lib/notmuch.h b/lib/notmuch.h > index f066245..c671d82 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -230,6 +230,16 @@ notmuch_status_t > notmuch_database_create (const char *path, notmuch_database_t **database); > > /** > + * Like notmuch_database_create, except optionally return an error > + * message. This message is allocated by malloc and should be freed by > + * the caller. > + */ > +notmuch_status_t > +notmuch_database_create_verbose (const char *path, > + notmuch_database_t **database, > + char **error_message); > + > +/** > * Database open mode for notmuch_database_open. > */ > typedef enum { > @@ -279,6 +289,17 @@ notmuch_status_t > notmuch_database_open (const char *path, > notmuch_database_mode_t mode, > notmuch_database_t **database); > +/** > + * Like notmuch_database_open, except optionally return an error > + * message. This message is allocated by malloc and should be freed by > + * the caller. > + */ > + > +notmuch_status_t > +notmuch_database_open_verbose (const char *path, > + notmuch_database_mode_t mode, > + notmuch_database_t **database, > + char **error_message); > > /** > * Commit changes and close the given notmuch database. > diff --git a/notmuch-new.c b/notmuch-new.c > index ddf42c1..93b70bf 100644 > --- a/notmuch-new.c > +++ b/notmuch-new.c > @@ -985,9 +985,13 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) > return EXIT_FAILURE; > add_files_state.total_files = count; > } else { > - if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, > - ¬much)) > + char *status_string = NULL; > + if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, > + ¬much, &status_string)) { > + if (status_string) fputs (status_string, stderr); > + > return EXIT_FAILURE; > + } > > if (notmuch_database_needs_upgrade (notmuch)) { > time_t now = time (NULL); > diff --git a/notmuch-search.c b/notmuch-search.c > index a591d45..d012af3 100644 > --- a/notmuch-search.c > +++ b/notmuch-search.c > @@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar > { > char *query_str; > unsigned int i; > + char *status_string = NULL; > > switch (ctx->format_sel) { > case NOTMUCH_FORMAT_TEXT: > @@ -570,9 +571,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar > > notmuch_exit_if_unsupported_format (); > > - if (notmuch_database_open (notmuch_config_get_database_path (config), > - NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch)) > + if (notmuch_database_open_verbose ( > + notmuch_config_get_database_path (config), > + NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) { > + if (status_string) fputs (status_string, stderr); > return EXIT_FAILURE; > + } > > query_str = query_string_from_args (ctx->notmuch, argc, argv); > if (query_str == NULL) { > diff --git a/test/symbol-test.cc b/test/symbol-test.cc > index 3e96c03..9f8eea7 100644 > --- a/test/symbol-test.cc > +++ b/test/symbol-test.cc > @@ -5,7 +5,11 @@ > > int main() { > notmuch_database_t *notmuch; > - notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much); > + char *message = NULL; > + > + if (notmuch_database_open_verbose ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much, &message)) > + if (message) fputs (message, stderr); > + > > try { > (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN); > -- > 2.1.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 34+ messages in thread
* [Patch v4 5/9] lib/database: add field for last error string 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner ` (3 preceding siblings ...) 2015-03-14 17:02 ` [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-14 17:02 ` [Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t David Bremner ` (3 subsequent siblings) 8 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch 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 6d6fa2c..24243db 100644 --- a/lib/database-private.h +++ b/lib/database-private.h @@ -154,6 +154,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 b774edc..48a830f 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -873,6 +873,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] == '/') @@ -2558,3 +2559,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 c671d82..20c4e01 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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner ` (4 preceding siblings ...) 2015-03-14 17:02 ` [Patch v4 5/9] lib/database: add field for last error string David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-14 17:02 ` [Patch v4 7/9] lib: add private function to extract the database for a message David Bremner ` (2 subsequent siblings) 8 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch 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 48a830f..0409128 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -377,6 +377,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, ¬much->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 8a1f2fa..7cb6fd4 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -190,6 +190,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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v4 7/9] lib: add private function to extract the database for a message. 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner ` (5 preceding siblings ...) 2015-03-14 17:02 ` [Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-14 17:02 ` [Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log David Bremner 2015-03-14 17:02 ` [Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open David Bremner 8 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch 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 956a70a..b84e5e1 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1626,3 +1626,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 7cb6fd4..dc58a2f 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -472,6 +472,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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner ` (6 preceding siblings ...) 2015-03-14 17:02 ` [Patch v4 7/9] lib: add private function to extract the database for a message David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-14 17:02 ` [Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open David Bremner 8 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch 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 | 38 +++++++++++++++++++++----------------- lib/directory.cc | 4 ++-- lib/index.cc | 11 +++++++---- lib/message.cc | 6 +++--- lib/query.cc | 8 ++++---- test/T560-lib-error.sh | 6 +++++- 6 files changed, 42 insertions(+), 31 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 0409128..6828654 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -499,7 +499,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; @@ -732,7 +732,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; } @@ -1025,7 +1025,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()); } } @@ -1157,12 +1157,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; @@ -1182,20 +1182,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; @@ -1203,7 +1203,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; @@ -1214,6 +1214,10 @@ notmuch_database_compact (const char *path, if (notmuch) { notmuch_status_t ret2; + const char *str = notmuch_database_status_string (notmuch); + if (status_cb && str) + status_cb (str, closure); + ret2 = notmuch_database_destroy (notmuch); /* don't clobber previous error status */ @@ -1232,7 +1236,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 @@ -1510,7 +1514,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; @@ -1560,7 +1564,7 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch) try { (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred beginning transaction: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred beginning transaction: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -1594,7 +1598,7 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch) if (thresh && atoi (thresh) == 1) db->flush (); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred committing transaction: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred committing transaction: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -1840,7 +1844,7 @@ notmuch_database_get_directory (notmuch_database_t *notmuch, *directory = _notmuch_directory_create (notmuch, path, NOTMUCH_FIND_LOOKUP, &status); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred getting directory: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2422,7 +2426,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch, _notmuch_message_sync (message); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred adding message: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred adding message: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2514,7 +2518,7 @@ notmuch_database_find_message_by_filename (notmuch_database_t *notmuch, status = NOTMUCH_STATUS_OUT_OF_MEMORY; } } catch (const Xapian::Error &error) { - fprintf (stderr, "Error: A Xapian exception occurred finding message by filename: %s\n", + _notmuch_database_log (notmuch, "Error: A Xapian exception occurred finding message by filename: %s\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2567,7 +2571,7 @@ notmuch_database_get_all_tags (notmuch_database_t *db) _notmuch_string_list_sort (tags); return _notmuch_tags_create (db, tags); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred getting tags: %s.\n", + _notmuch_database_log (db, "A Xapian exception occurred getting tags: %s.\n", error.get_msg().c_str()); db->exception_reported = TRUE; return NULL; diff --git a/lib/directory.cc b/lib/directory.cc index 8daaec8..b836ea2 100644 --- a/lib/directory.cc +++ b/lib/directory.cc @@ -186,7 +186,7 @@ _notmuch_directory_create (notmuch_database_t *notmuch, directory->mtime = Xapian::sortable_unserialise ( directory->doc.get_value (NOTMUCH_VALUE_TIMESTAMP)); } catch (const Xapian::Error &error) { - fprintf (stderr, + _notmuch_database_log (notmuch, "A Xapian exception occurred creating a directory: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; @@ -228,7 +228,7 @@ notmuch_directory_set_mtime (notmuch_directory_t *directory, db->replace_document (directory->document_id, directory->doc); } catch (const Xapian::Error &error) { - fprintf (stderr, + _notmuch_database_log (notmuch, "A Xapian exception occurred setting directory mtime: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; diff --git a/lib/index.cc b/lib/index.cc index c88ed8d..e81aa81 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -314,7 +314,8 @@ _index_mime_part (notmuch_message_t *message, const char *charset; if (! part) { - fprintf (stderr, "Warning: Not indexing empty mime part.\n"); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Not indexing empty mime part.\n"); return; } @@ -344,7 +345,8 @@ _index_mime_part (notmuch_message_t *message, if (i == 1) continue; if (i > 1) - fprintf (stderr, "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n"); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n"); } if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) { /* Don't index encrypted parts. */ @@ -367,8 +369,9 @@ _index_mime_part (notmuch_message_t *message, } if (! (GMIME_IS_PART (part))) { - fprintf (stderr, "Warning: Not indexing unknown mime part: %s.\n", - g_type_name (G_OBJECT_TYPE (part))); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Not indexing unknown mime part: %s.\n", + g_type_name (G_OBJECT_TYPE (part))); return; } diff --git a/lib/message.cc b/lib/message.cc index b84e5e1..a8ca988 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -252,7 +252,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch, doc_id = _notmuch_database_generate_doc_id (notmuch); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred creating message: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred creating message: %s\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; *status_ret = NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION; @@ -467,7 +467,7 @@ notmuch_message_get_header (notmuch_message_t *message, const char *header) return talloc_strdup (message, value.c_str ()); } catch (Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred when reading header: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading header: %s\n", error.get_msg().c_str()); message->notmuch->exception_reported = TRUE; return NULL; @@ -921,7 +921,7 @@ notmuch_message_get_date (notmuch_message_t *message) try { value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP); } catch (Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred when reading date: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading date: %s\n", error.get_msg().c_str()); message->notmuch->exception_reported = TRUE; return 0; diff --git a/lib/query.cc b/lib/query.cc index 57aa6d2..7b59786 100644 --- a/lib/query.cc +++ b/lib/query.cc @@ -296,9 +296,9 @@ notmuch_query_search_messages_st (notmuch_query_t *query, return NOTMUCH_STATUS_SUCCESS; } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred performing query: %s\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred performing query: %s\n", error.get_msg().c_str()); - fprintf (stderr, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string); notmuch->exception_reported = TRUE; talloc_free (messages); return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -597,9 +597,9 @@ notmuch_query_count_messages (notmuch_query_t *query) count = mset.get_matches_estimated(); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred: %s\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred: %s\n", error.get_msg().c_str()); - fprintf (stderr, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string); } return count; diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh index dc2022f..7621e7f 100755 --- a/test/T560-lib-error.sh +++ b/test/T560-lib-error.sh @@ -56,6 +56,9 @@ int main (int argc, char** argv) fprintf (stderr, "error opening database: %d\n", stat); } stat = notmuch_database_add_message (db, "/dev/null", NULL); + if (stat) + fputs (notmuch_database_status_string (db), stderr); + } EOF cat <<EOF >EXPECTED @@ -83,8 +86,9 @@ int main (int argc, char** argv) EOF cat <<EOF >EXPECTED == stdout == -== stderr == Path already exists: CWD/mail + +== stderr == EOF test_expect_equal_file EXPECTED OUTPUT -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open 2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner ` (7 preceding siblings ...) 2015-03-14 17:02 ` [Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log David Bremner @ 2015-03-14 17:02 ` David Bremner 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner 8 siblings, 2 replies; 34+ messages in thread From: David Bremner @ 2015-03-14 17:02 UTC (permalink / raw) To: notmuch 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 6828654..b5f3549 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -2317,7 +2317,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 a41d9ad..8ac96e8 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 a8ca988..5bc7aff 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 dc58a2f..cc9ce12 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -358,11 +358,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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open 2015-03-14 17:02 ` [Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open David Bremner @ 2015-03-24 13:19 ` David Bremner 2015-03-24 13:19 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner ` (6 more replies) 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner 1 sibling, 7 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch This is arguably testing the same thing twice, but in the brave new future where we don't use printf anymore, each subcommand will be responsible for handling the output on it's own. --- test/T050-new.sh | 7 +++++++ test/T150-tagging.sh | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/test/T050-new.sh b/test/T050-new.sh index 7119356..e6c3291 100755 --- a/test/T050-new.sh +++ b/test/T050-new.sh @@ -276,4 +276,11 @@ test_expect_code 1 "Invalid tags set exit code" \ notmuch config set new.tags $OLDCONFIG + +test_begin_subtest "Xapian exception: read only files" +chmod u-w ${MAIL_DIR}/.notmuch/xapian/*.DB +output=$(NOTMUCH_NEW 2>&1 | sed 's/: .*$//' ) +chmod u+w ${MAIL_DIR}/.notmuch/xapian/*.DB +test_expect_equal "$output" "A Xapian exception occurred opening database" + test_done diff --git a/test/T150-tagging.sh b/test/T150-tagging.sh index 45471ac..4a2673d 100755 --- a/test/T150-tagging.sh +++ b/test/T150-tagging.sh @@ -261,4 +261,10 @@ test_expect_code 1 "Empty tag names" 'notmuch tag + One' test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One' +test_begin_subtest "Xapian exception: read only files" +chmod u-w ${MAIL_DIR}/.notmuch/xapian/*.DB +output=$(notmuch tag +something '*' 2>&1 | sed 's/: .*$//' ) +chmod u+w ${MAIL_DIR}/.notmuch/xapian/*.DB +test_expect_equal "$output" "A Xapian exception occurred opening database" + test_done -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 2/8] test: add support for compiling and running C snippets 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner @ 2015-03-24 13:19 ` David Bremner 2015-03-24 13:19 ` [Patch v5 3/8] test: add error reporting tests David Bremner ` (5 subsequent siblings) 6 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch This is to limit the copy-pasta involved in running C tests. I decided to keep things simple and not try to provide an actual C skeleton. The setting of LD_LIBRARY_PATH is to force using the built libnotmuch rather than any potential system one. --- test/README | 5 +++++ test/test-lib.sh | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/test/README b/test/README index 81a1c82..5b40474 100644 --- a/test/README +++ b/test/README @@ -84,6 +84,11 @@ the tests in one of the following ways. TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient +Some tests may require a c compiler. You can choose the name and flags similarly +to with emacs, e.g. + + make test TEST_CC=gcc TEST_CFLAGS="-g -O2" + Quiet Execution --------------- diff --git a/test/test-lib.sh b/test/test-lib.sh index 133fbe4..fdb84ea 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -73,6 +73,8 @@ if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \ fi TEST_EMACS=${TEST_EMACS:-${EMACS:-emacs}} TEST_EMACSCLIENT=${TEST_EMACSCLIENT:-emacsclient} +TEST_CC=${TEST_CC:-cc} +TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"} # Protect ourselves from common misconfiguration to export # CDPATH into the environment @@ -1161,6 +1163,19 @@ test_python() { | $cmd - } +test_C () { + exec_file="test${test_count}" + test_file="${exec_file}.c" + cat > ${test_file} + export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib + ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch -ltalloc + echo "== stdout ==" > OUTPUT.stdout + echo "== stderr ==" > OUTPUT.stderr + ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr + sed "s,$(pwd),CWD," OUTPUT.stdout OUTPUT.stderr > OUTPUT +} + + # Creates a script that counts how much time it is executed and calls # notmuch. $notmuch_counter_command is set to the path to the # generated script. Use notmuch_counter_value() function to get the -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 3/8] test: add error reporting tests 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner 2015-03-24 13:19 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner @ 2015-03-24 13:19 ` David Bremner 2015-03-24 13:19 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner ` (4 subsequent siblings) 6 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch This includes tests for all of the error fprintfs in the library I could figure out how to test without using gdb scripts. It boils down to errors opening files and exceptions caused by corrupted Xapian databases. --- test/T560-lib-error.sh | 250 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) create mode 100755 test/T560-lib-error.sh diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh new file mode 100755 index 0000000..c2964cb --- /dev/null +++ b/test/T560-lib-error.sh @@ -0,0 +1,250 @@ +#!/usr/bin/env bash +test_description="error reporting for library" + +. ./test-lib.sh + +backup_database (){ + rm -rf notmuch-dir-backup + cp -a ${MAIL_DIR}/.notmuch notmuch-dir-backup +} +restore_database (){ + rm -rf ${MAIL_DIR}/.notmuch + cp -a notmuch-dir-backup ${MAIL_DIR}/.notmuch +} + + +add_email_corpus + +test_expect_success "building database" "NOTMUCH_NEW" + +test_begin_subtest "Open null pointer" +test_C <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (NULL, 0, 0); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error: Cannot open a database for a NULL path. +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Open nonexistent database" +test_C <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open ("/nonexistent/foo", 0, 0); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error opening database at /nonexistent/foo/.notmuch: No such file or directory +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Write to read-only database" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_ONLY, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + stat = notmuch_database_add_message (db, "/dev/null", NULL); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Cannot write to a read-only database. +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Add non-existent file" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + stat = notmuch_database_add_message (db, "/nonexistent", NULL); + if (stat) + fputs (notmuch_database_status_string (db), stderr); + +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error opening /nonexistent: No such file or directory +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "compact, overwriting existing backup" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +static void +status_cb (const char *msg, void *closure) +{ + printf ("%s\n", msg); +} +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_compact (argv[1], argv[1], status_cb, NULL); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Path already exists: CWD/mail +EOF +test_expect_equal_file EXPECTED OUTPUT + +cat <<EOF > head.c +#include <stdio.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <talloc.h> +#include <notmuch.h> + +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + char *path; + int fd; + + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + path = talloc_asprintf (db, "%s/.notmuch/xapian/postlist.DB", argv[1]); + fd = open(path,O_WRONLY|O_TRUNC); + if (fd < 0) + fprintf (stderr, "error opening %s\n"); +EOF +cat <<EOF > tail.c + if (stat) { + const char *stat_str = notmuch_database_status_string (db); + if (stat_str) + fputs (stat_str, stderr); + } + +} +EOF + +backup_database +test_begin_subtest "Xapian exception finding message" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_message_t *message = NULL; + stat = notmuch_database_find_message (db, "id:nonexistant", &message); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred finding message +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception getting tags" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_tags_t *tags = NULL; + tags = notmuch_database_get_all_tags (db); + stat = (tags == NULL); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred getting tags +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception creating directory" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_directory_t *directory = NULL; + stat = notmuch_database_get_directory (db, "none/existing", &directory); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred creating a directory +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception searching messages" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_messages_t *messages = NULL; + notmuch_query_t *query=notmuch_query_create (db, "*"); + stat = notmuch_query_search_messages_st (query, &messages); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred performing query +Query string was: * +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception counting messages" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_query_t *query=notmuch_query_create (db, "id:87ocn0qh6d.fsf@yoom.home.cworth.org"); + int count = notmuch_query_count_messages (query); + stat = (count == 0); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred performing query +Query string was: id:87ocn0qh6d.fsf@yoom.home.cworth.org +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +test_done -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner 2015-03-24 13:19 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner 2015-03-24 13:19 ` [Patch v5 3/8] test: add error reporting tests David Bremner @ 2015-03-24 13:19 ` David Bremner 2015-03-24 13:19 ` [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t David Bremner ` (3 subsequent siblings) 6 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch The compatibility wrapper ensures that clients calling notmuch_database_open will receive consistent output for now. The changes to notmuch-{new,search} and test/symbol-test are just to make the test suite pass. The use of IGNORE_RESULT is justified by two things. 1) I don't know what else to do. 2) asprintf guarantees the output string is NULL if an error occurs, so at least we are not passing garbage back. --- lib/database.cc | 94 +++++++++++++++++++++++++++++++++++++---------------- lib/notmuch.h | 21 ++++++++++++ notmuch-new.c | 8 +++-- notmuch-search.c | 8 +++-- test/symbol-test.cc | 6 +++- 5 files changed, 104 insertions(+), 33 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 3974e2e..36849d7 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -608,29 +608,39 @@ 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"); + message = strdup ("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", - path, strerror (errno)); + IGNORE_RESULT (asprintf (&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", - path); + IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: " + "Not a directory.\n", + path)); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } @@ -640,15 +650,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", - notmuch_path, strerror (errno)); + IGNORE_RESULT (asprintf (&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, - ¬much); + status = notmuch_database_open_verbose (path, + NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, &message); if (status) goto DONE; @@ -667,6 +677,8 @@ notmuch_database_create (const char *path, notmuch_database_t **database) if (notmuch_path) talloc_free (notmuch_path); + if (message) + *status_string = message; if (database) *database = notmuch; else @@ -767,37 +779,55 @@ notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database) { + char *status_string = NULL; + notmuch_status_t status; + + status = notmuch_database_open_verbose(path, mode, database, + &status_string); + + if (status_string) fputs(status_string, stderr); + + return status; +} + +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, "Error: Cannot open a database for a NULL path.\n"); + message = strdup ("Error: Cannot open a database for a NULL path.\n"); status = NOTMUCH_STATUS_NULL_POINTER; goto DONE; } if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) { - fprintf (stderr, "Out of memory\n"); + message = strdup ("Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } err = stat (notmuch_path, &st); if (err) { - fprintf (stderr, "Error opening database at %s: %s\n", - notmuch_path, strerror (errno)); + IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n", + notmuch_path, strerror (errno))); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { - fprintf (stderr, "Out of memory\n"); + message = strdup ("Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } @@ -837,11 +867,11 @@ notmuch_database_open (const char *path, * means a dramatically incompatible change. */ version = notmuch_database_get_version (notmuch); if (version > NOTMUCH_DATABASE_VERSION) { - fprintf (stderr, - "Error: Notmuch database at %s\n" - " has a newer database format version (%u) than supported by this\n" - " version of notmuch (%u).\n", - notmuch_path, version, NOTMUCH_DATABASE_VERSION); + IGNORE_RESULT (asprintf (&message, + "Error: Notmuch database at %s\n" + " has a newer database format version (%u) than supported by this\n" + " version of notmuch (%u).\n", + notmuch_path, version, NOTMUCH_DATABASE_VERSION)); notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; notmuch_database_destroy (notmuch); notmuch = NULL; @@ -856,11 +886,11 @@ notmuch_database_open (const char *path, version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r', &incompat_features); if (incompat_features) { - fprintf (stderr, - "Error: Notmuch database at %s\n" - " requires features (%s)\n" - " not supported by this version of notmuch.\n", - notmuch_path, incompat_features); + IGNORE_RESULT (asprintf (&message, + "Error: Notmuch database at %s\n" + " requires features (%s)\n" + " not supported by this version of notmuch.\n", + notmuch_path, incompat_features)); notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; notmuch_database_destroy (notmuch); notmuch = NULL; @@ -906,8 +936,8 @@ notmuch_database_open (const char *path, notmuch->query_parser->add_prefix (prefix->name, prefix->prefix); } } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred opening database: %s\n", - error.get_msg().c_str()); + IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n", + error.get_msg().c_str())); notmuch_database_destroy (notmuch); notmuch = NULL; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -916,6 +946,9 @@ notmuch_database_open (const char *path, DONE: talloc_free (local); + if (status_string && message) + *status_string = message; + if (database) *database = notmuch; else @@ -1039,13 +1072,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, ¬much); + ret = notmuch_database_open_verbose (path, + NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, + &message); if (ret) { + if (status_cb) status_cb (message, closure); goto DONE; } diff --git a/lib/notmuch.h b/lib/notmuch.h index f066245..c671d82 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -230,6 +230,16 @@ notmuch_status_t notmuch_database_create (const char *path, notmuch_database_t **database); /** + * Like notmuch_database_create, except optionally return an error + * message. This message is allocated by malloc and should be freed by + * the caller. + */ +notmuch_status_t +notmuch_database_create_verbose (const char *path, + notmuch_database_t **database, + char **error_message); + +/** * Database open mode for notmuch_database_open. */ typedef enum { @@ -279,6 +289,17 @@ notmuch_status_t notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database); +/** + * Like notmuch_database_open, except optionally return an error + * message. This message is allocated by malloc and should be freed by + * the caller. + */ + +notmuch_status_t +notmuch_database_open_verbose (const char *path, + notmuch_database_mode_t mode, + notmuch_database_t **database, + char **error_message); /** * Commit changes and close the given notmuch database. diff --git a/notmuch-new.c b/notmuch-new.c index ddf42c1..93b70bf 100644 --- a/notmuch-new.c +++ b/notmuch-new.c @@ -985,9 +985,13 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) return EXIT_FAILURE; add_files_state.total_files = count; } else { - if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, - ¬much)) + char *status_string = NULL; + if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, &status_string)) { + if (status_string) fputs (status_string, stderr); + return EXIT_FAILURE; + } if (notmuch_database_needs_upgrade (notmuch)) { time_t now = time (NULL); diff --git a/notmuch-search.c b/notmuch-search.c index a591d45..d012af3 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar { char *query_str; unsigned int i; + char *status_string = NULL; switch (ctx->format_sel) { case NOTMUCH_FORMAT_TEXT: @@ -570,9 +571,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar notmuch_exit_if_unsupported_format (); - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch)) + if (notmuch_database_open_verbose ( + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) { + if (status_string) fputs (status_string, stderr); return EXIT_FAILURE; + } query_str = query_string_from_args (ctx->notmuch, argc, argv); if (query_str == NULL) { diff --git a/test/symbol-test.cc b/test/symbol-test.cc index 3e96c03..9f8eea7 100644 --- a/test/symbol-test.cc +++ b/test/symbol-test.cc @@ -5,7 +5,11 @@ int main() { notmuch_database_t *notmuch; - notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much); + char *message = NULL; + + if (notmuch_database_open_verbose ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much, &message)) + if (message) fputs (message, stderr); + try { (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN); -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner ` (2 preceding siblings ...) 2015-03-24 13:19 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner @ 2015-03-24 13:19 ` David Bremner 2015-03-24 13:19 ` [Patch v5 6/8] lib: add private function to extract the database for a message David Bremner ` (2 subsequent siblings) 6 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch In principle in the future this could do something fancier than sprintf. --- lib/database-private.h | 4 ++++ lib/database.cc | 24 ++++++++++++++++++++++++ lib/notmuch-private.h | 4 ++++ lib/notmuch.h | 7 +++++++ 4 files changed, 39 insertions(+) diff --git a/lib/database-private.h b/lib/database-private.h index 6d6fa2c..24243db 100644 --- a/lib/database-private.h +++ b/lib/database-private.h @@ -154,6 +154,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 36849d7..673561b 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -348,6 +348,23 @@ notmuch_status_to_string (notmuch_status_t status) } } +void +_notmuch_database_log (notmuch_database_t *notmuch, + const char *format, + ...) +{ + va_list va_args; + + va_start (va_args, format); + + if (notmuch->status_string) + talloc_free (notmuch->status_string); + + notmuch->status_string = talloc_vasprintf (notmuch, format, va_args); + + va_end (va_args); +} + static void find_doc_ids_for_term (notmuch_database_t *notmuch, const char *term, @@ -845,6 +862,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] == '/') @@ -2530,3 +2548,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-private.h b/lib/notmuch-private.h index 8a1f2fa..7cb6fd4 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -190,6 +190,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); diff --git a/lib/notmuch.h b/lib/notmuch.h index c671d82..20c4e01 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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 6/8] lib: add private function to extract the database for a message. 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner ` (3 preceding siblings ...) 2015-03-24 13:19 ` [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t David Bremner @ 2015-03-24 13:19 ` David Bremner 2015-03-24 13:19 ` [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log David Bremner 2015-03-24 13:19 ` [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open David Bremner 6 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch 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 956a70a..b84e5e1 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1626,3 +1626,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 7cb6fd4..dc58a2f 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -472,6 +472,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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner ` (4 preceding siblings ...) 2015-03-24 13:19 ` [Patch v5 6/8] lib: add private function to extract the database for a message David Bremner @ 2015-03-24 13:19 ` David Bremner 2015-03-24 13:19 ` [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open David Bremner 6 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch 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 | 38 +++++++++++++++++++++----------------- lib/directory.cc | 4 ++-- lib/index.cc | 11 +++++++---- lib/message.cc | 6 +++--- lib/query.cc | 18 ++++++++++++------ test/T560-lib-error.sh | 6 +++++- 6 files changed, 50 insertions(+), 33 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 673561b..155b5aa 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -473,7 +473,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; @@ -707,7 +707,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; } @@ -1000,7 +1000,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()); } } @@ -1132,12 +1132,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; @@ -1157,20 +1157,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; @@ -1178,7 +1178,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; @@ -1189,6 +1189,10 @@ notmuch_database_compact (const char *path, if (notmuch) { notmuch_status_t ret2; + const char *str = notmuch_database_status_string (notmuch); + if (status_cb && str) + status_cb (str, closure); + ret2 = notmuch_database_destroy (notmuch); /* don't clobber previous error status */ @@ -1207,7 +1211,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 @@ -1485,7 +1489,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; @@ -1535,7 +1539,7 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch) try { (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred beginning transaction: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred beginning transaction: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -1569,7 +1573,7 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch) if (thresh && atoi (thresh) == 1) db->flush (); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred committing transaction: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred committing transaction: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -1815,7 +1819,7 @@ notmuch_database_get_directory (notmuch_database_t *notmuch, *directory = _notmuch_directory_create (notmuch, path, NOTMUCH_FIND_LOOKUP, &status); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred getting directory: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2397,7 +2401,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch, _notmuch_message_sync (message); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred adding message: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred adding message: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2489,7 +2493,7 @@ notmuch_database_find_message_by_filename (notmuch_database_t *notmuch, status = NOTMUCH_STATUS_OUT_OF_MEMORY; } } catch (const Xapian::Error &error) { - fprintf (stderr, "Error: A Xapian exception occurred finding message by filename: %s\n", + _notmuch_database_log (notmuch, "Error: A Xapian exception occurred finding message by filename: %s\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2542,7 +2546,7 @@ notmuch_database_get_all_tags (notmuch_database_t *db) _notmuch_string_list_sort (tags); return _notmuch_tags_create (db, tags); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred getting tags: %s.\n", + _notmuch_database_log (db, "A Xapian exception occurred getting tags: %s.\n", error.get_msg().c_str()); db->exception_reported = TRUE; return NULL; diff --git a/lib/directory.cc b/lib/directory.cc index 8daaec8..b836ea2 100644 --- a/lib/directory.cc +++ b/lib/directory.cc @@ -186,7 +186,7 @@ _notmuch_directory_create (notmuch_database_t *notmuch, directory->mtime = Xapian::sortable_unserialise ( directory->doc.get_value (NOTMUCH_VALUE_TIMESTAMP)); } catch (const Xapian::Error &error) { - fprintf (stderr, + _notmuch_database_log (notmuch, "A Xapian exception occurred creating a directory: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; @@ -228,7 +228,7 @@ notmuch_directory_set_mtime (notmuch_directory_t *directory, db->replace_document (directory->document_id, directory->doc); } catch (const Xapian::Error &error) { - fprintf (stderr, + _notmuch_database_log (notmuch, "A Xapian exception occurred setting directory mtime: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; diff --git a/lib/index.cc b/lib/index.cc index c88ed8d..e81aa81 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -314,7 +314,8 @@ _index_mime_part (notmuch_message_t *message, const char *charset; if (! part) { - fprintf (stderr, "Warning: Not indexing empty mime part.\n"); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Not indexing empty mime part.\n"); return; } @@ -344,7 +345,8 @@ _index_mime_part (notmuch_message_t *message, if (i == 1) continue; if (i > 1) - fprintf (stderr, "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n"); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n"); } if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) { /* Don't index encrypted parts. */ @@ -367,8 +369,9 @@ _index_mime_part (notmuch_message_t *message, } if (! (GMIME_IS_PART (part))) { - fprintf (stderr, "Warning: Not indexing unknown mime part: %s.\n", - g_type_name (G_OBJECT_TYPE (part))); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Not indexing unknown mime part: %s.\n", + g_type_name (G_OBJECT_TYPE (part))); return; } diff --git a/lib/message.cc b/lib/message.cc index b84e5e1..a8ca988 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -252,7 +252,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch, doc_id = _notmuch_database_generate_doc_id (notmuch); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred creating message: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred creating message: %s\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; *status_ret = NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION; @@ -467,7 +467,7 @@ notmuch_message_get_header (notmuch_message_t *message, const char *header) return talloc_strdup (message, value.c_str ()); } catch (Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred when reading header: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading header: %s\n", error.get_msg().c_str()); message->notmuch->exception_reported = TRUE; return NULL; @@ -921,7 +921,7 @@ notmuch_message_get_date (notmuch_message_t *message) try { value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP); } catch (Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred when reading date: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading date: %s\n", error.get_msg().c_str()); message->notmuch->exception_reported = TRUE; return 0; diff --git a/lib/query.cc b/lib/query.cc index 57aa6d2..9cedb6a 100644 --- a/lib/query.cc +++ b/lib/query.cc @@ -296,9 +296,12 @@ notmuch_query_search_messages_st (notmuch_query_t *query, return NOTMUCH_STATUS_SUCCESS; } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred performing query: %s\n", - error.get_msg().c_str()); - fprintf (stderr, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, + "A Xapian exception occurred performing query: %s\n" + "Query string was: %s\n", + error.get_msg().c_str(), + query->query_string); + notmuch->exception_reported = TRUE; talloc_free (messages); return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -597,9 +600,12 @@ notmuch_query_count_messages (notmuch_query_t *query) count = mset.get_matches_estimated(); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred: %s\n", - error.get_msg().c_str()); - fprintf (stderr, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, + "A Xapian exception occurred performing query: %s\n" + "Query string was: %s\n", + error.get_msg().c_str(), + query->query_string); + } return count; diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh index c2964cb..ec7552a 100755 --- a/test/T560-lib-error.sh +++ b/test/T560-lib-error.sh @@ -66,6 +66,9 @@ int main (int argc, char** argv) fprintf (stderr, "error opening database: %d\n", stat); } stat = notmuch_database_add_message (db, "/dev/null", NULL); + if (stat) + fputs (notmuch_database_status_string (db), stderr); + } EOF cat <<EOF >EXPECTED @@ -118,8 +121,9 @@ int main (int argc, char** argv) EOF cat <<EOF >EXPECTED == stdout == -== stderr == Path already exists: CWD/mail + +== stderr == EOF test_expect_equal_file EXPECTED OUTPUT -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner ` (5 preceding siblings ...) 2015-03-24 13:19 ` [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log David Bremner @ 2015-03-24 13:19 ` David Bremner 6 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:19 UTC (permalink / raw) To: David Bremner, notmuch 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. --- 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 155b5aa..85054df 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -2292,7 +2292,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 a41d9ad..8ac96e8 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 a8ca988..5bc7aff 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 dc58a2f..cc9ce12 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -358,11 +358,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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Update to library logging, version 5 2015-03-14 17:02 ` [Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open David Bremner 2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-24 13:24 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner ` (7 more replies) 1 sibling, 8 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch This takes into account (most of) Tomi's comments and adds a bunch more tests. We bikeshedded a bit about log_to_string on IRC, and eventually convered on eliminating it. I guess it might be necessary to add a compat version of asprintf for some environments (Solaris?) but this is easy enough using talloc_asprintf and strdup Here is the diff between the two versions diff --git a/lib/database.cc b/lib/database.cc index b5f3549..85054df 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -348,35 +348,6 @@ 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); -} - void _notmuch_database_log (notmuch_database_t *notmuch, const char *format, @@ -386,7 +357,10 @@ _notmuch_database_log (notmuch_database_t *notmuch, va_start (va_args, format); - vlog_to_string (notmuch, ¬much->status_string, format, va_args); + if (notmuch->status_string) + talloc_free (notmuch->status_string); + + notmuch->status_string = talloc_vasprintf (notmuch, format, va_args); va_end (va_args); } @@ -667,22 +641,23 @@ notmuch_database_create_verbose (const char *path, int err; if (path == NULL) { - log_to_string (&message, "Error: Cannot create a database for a NULL path.\n"); + message = strdup ("Error: Cannot create a database for a NULL path.\n"); status = NOTMUCH_STATUS_NULL_POINTER; goto DONE; } err = stat (path, &st); if (err) { - log_to_string (&message, "Error: Cannot create database at %s: %s.\n", - path, strerror (errno)); + IGNORE_RESULT (asprintf (&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)) { - log_to_string (&message, "Error: Cannot create database at %s: Not a directory.\n", - path); + IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: " + "Not a directory.\n", + path)); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } @@ -692,8 +667,8 @@ notmuch_database_create_verbose (const char *path, err = mkdir (notmuch_path, 0755); if (err) { - log_to_string (&message, "Error: Cannot create directory %s: %s.\n", - notmuch_path, strerror (errno)); + IGNORE_RESULT (asprintf (&message, "Error: Cannot create directory %s: %s.\n", + notmuch_path, strerror (errno))); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } @@ -720,7 +695,7 @@ notmuch_database_create_verbose (const char *path, talloc_free (notmuch_path); if (message) - *status_string = strdup(message); + *status_string = message; if (database) *database = notmuch; else @@ -849,27 +824,27 @@ notmuch_database_open_verbose (const char *path, static int initialized = 0; if (path == NULL) { - log_to_string (&message, "Error: Cannot open a database for a NULL path.\n"); + message = strdup ("Error: Cannot open a database for a NULL path.\n"); status = NOTMUCH_STATUS_NULL_POINTER; goto DONE; } if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) { - log_to_string (&message, "Out of memory\n"); + message = strdup ("Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } err = stat (notmuch_path, &st); if (err) { - log_to_string (&message, "Error opening database at %s: %s\n", - notmuch_path, strerror (errno)); + IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n", + notmuch_path, strerror (errno))); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { - log_to_string (&message, "Out of memory\n"); + message = strdup ("Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } @@ -910,11 +885,11 @@ notmuch_database_open_verbose (const char *path, * means a dramatically incompatible change. */ version = notmuch_database_get_version (notmuch); if (version > NOTMUCH_DATABASE_VERSION) { - log_to_string (&message, - "Error: Notmuch database at %s\n" - " has a newer database format version (%u) than supported by this\n" - " version of notmuch (%u).\n", - notmuch_path, version, NOTMUCH_DATABASE_VERSION); + IGNORE_RESULT (asprintf (&message, + "Error: Notmuch database at %s\n" + " has a newer database format version (%u) than supported by this\n" + " version of notmuch (%u).\n", + notmuch_path, version, NOTMUCH_DATABASE_VERSION)); notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; notmuch_database_destroy (notmuch); notmuch = NULL; @@ -929,11 +904,11 @@ notmuch_database_open_verbose (const char *path, version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r', &incompat_features); if (incompat_features) { - log_to_string (&message, - "Error: Notmuch database at %s\n" - " requires features (%s)\n" - " not supported by this version of notmuch.\n", - notmuch_path, incompat_features); + IGNORE_RESULT (asprintf (&message, + "Error: Notmuch database at %s\n" + " requires features (%s)\n" + " not supported by this version of notmuch.\n", + notmuch_path, incompat_features)); notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; notmuch_database_destroy (notmuch); notmuch = NULL; @@ -979,8 +954,8 @@ notmuch_database_open_verbose (const char *path, notmuch->query_parser->add_prefix (prefix->name, prefix->prefix); } } catch (const Xapian::Error &error) { - log_to_string (&message, "A Xapian exception occurred opening database: %s\n", - error.get_msg().c_str()); + IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n", + error.get_msg().c_str())); notmuch_database_destroy (notmuch); notmuch = NULL; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -990,7 +965,7 @@ notmuch_database_open_verbose (const char *path, talloc_free (local); if (status_string && message) - *status_string = strdup (message); + *status_string = message; if (database) *database = notmuch; diff --git a/lib/query.cc b/lib/query.cc index 7b59786..9cedb6a 100644 --- a/lib/query.cc +++ b/lib/query.cc @@ -296,9 +296,12 @@ notmuch_query_search_messages_st (notmuch_query_t *query, return NOTMUCH_STATUS_SUCCESS; } catch (const Xapian::Error &error) { - _notmuch_database_log (notmuch, "A Xapian exception occurred performing query: %s\n", - error.get_msg().c_str()); - _notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, + "A Xapian exception occurred performing query: %s\n" + "Query string was: %s\n", + error.get_msg().c_str(), + query->query_string); + notmuch->exception_reported = TRUE; talloc_free (messages); return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -597,9 +600,12 @@ notmuch_query_count_messages (notmuch_query_t *query) count = mset.get_matches_estimated(); } catch (const Xapian::Error &error) { - _notmuch_database_log (notmuch, "A Xapian exception occurred: %s\n", - error.get_msg().c_str()); - _notmuch_database_log (notmuch, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, + "A Xapian exception occurred performing query: %s\n" + "Query string was: %s\n", + error.get_msg().c_str(), + query->query_string); + } return count; diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh index 7621e7f..ec7552a 100755 --- a/test/T560-lib-error.sh +++ b/test/T560-lib-error.sh @@ -3,6 +3,16 @@ test_description="error reporting for library" . ./test-lib.sh +backup_database (){ + rm -rf notmuch-dir-backup + cp -a ${MAIL_DIR}/.notmuch notmuch-dir-backup +} +restore_database (){ + rm -rf ${MAIL_DIR}/.notmuch + cp -a notmuch-dir-backup ${MAIL_DIR}/.notmuch +} + + add_email_corpus test_expect_success "building database" "NOTMUCH_NEW" @@ -68,6 +78,31 @@ Cannot write to a read-only database. EOF test_expect_equal_file EXPECTED OUTPUT +test_begin_subtest "Add non-existent file" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + stat = notmuch_database_add_message (db, "/nonexistent", NULL); + if (stat) + fputs (notmuch_database_status_string (db), stderr); + +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error opening /nonexistent: No such file or directory +EOF +test_expect_equal_file EXPECTED OUTPUT + test_begin_subtest "compact, overwriting existing backup" test_C ${MAIL_DIR} <<EOF #include <stdio.h> @@ -92,4 +127,128 @@ Path already exists: CWD/mail EOF test_expect_equal_file EXPECTED OUTPUT +cat <<EOF > head.c +#include <stdio.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <talloc.h> +#include <notmuch.h> + +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + char *path; + int fd; + + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + path = talloc_asprintf (db, "%s/.notmuch/xapian/postlist.DB", argv[1]); + fd = open(path,O_WRONLY|O_TRUNC); + if (fd < 0) + fprintf (stderr, "error opening %s\n"); +EOF +cat <<EOF > tail.c + if (stat) { + const char *stat_str = notmuch_database_status_string (db); + if (stat_str) + fputs (stat_str, stderr); + } + +} +EOF + +backup_database +test_begin_subtest "Xapian exception finding message" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_message_t *message = NULL; + stat = notmuch_database_find_message (db, "id:nonexistant", &message); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred finding message +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception getting tags" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_tags_t *tags = NULL; + tags = notmuch_database_get_all_tags (db); + stat = (tags == NULL); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred getting tags +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception creating directory" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_directory_t *directory = NULL; + stat = notmuch_database_get_directory (db, "none/existing", &directory); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred creating a directory +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception searching messages" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_messages_t *messages = NULL; + notmuch_query_t *query=notmuch_query_create (db, "*"); + stat = notmuch_query_search_messages_st (query, &messages); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred performing query +Query string was: * +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception counting messages" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_query_t *query=notmuch_query_create (db, "id:87ocn0qh6d.fsf@yoom.home.cworth.org"); + int count = notmuch_query_count_messages (query); + stat = (count == 0); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred performing query +Query string was: id:87ocn0qh6d.fsf@yoom.home.cworth.org +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + test_done diff --git a/test/test-lib.sh b/test/test-lib.sh index c7af003..fdb84ea 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -1163,16 +1163,16 @@ test_python() { | $cmd - } -test_C() { - test_file="test${test_count}.c" - exec_file=${test_file%%.c} +test_C () { + exec_file="test${test_count}" + test_file="${exec_file}.c" cat > ${test_file} export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib - ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch + ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch -ltalloc echo "== stdout ==" > OUTPUT.stdout echo "== stderr ==" > OUTPUT.stderr ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr - cat OUTPUT.stdout OUTPUT.stderr | sed "s,$(pwd),CWD," > OUTPUT + sed "s,$(pwd),CWD," OUTPUT.stdout OUTPUT.stderr > OUTPUT } ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-24 13:24 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner ` (6 subsequent siblings) 7 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch This is arguably testing the same thing twice, but in the brave new future where we don't use printf anymore, each subcommand will be responsible for handling the output on it's own. --- test/T050-new.sh | 7 +++++++ test/T150-tagging.sh | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/test/T050-new.sh b/test/T050-new.sh index 7119356..e6c3291 100755 --- a/test/T050-new.sh +++ b/test/T050-new.sh @@ -276,4 +276,11 @@ test_expect_code 1 "Invalid tags set exit code" \ notmuch config set new.tags $OLDCONFIG + +test_begin_subtest "Xapian exception: read only files" +chmod u-w ${MAIL_DIR}/.notmuch/xapian/*.DB +output=$(NOTMUCH_NEW 2>&1 | sed 's/: .*$//' ) +chmod u+w ${MAIL_DIR}/.notmuch/xapian/*.DB +test_expect_equal "$output" "A Xapian exception occurred opening database" + test_done diff --git a/test/T150-tagging.sh b/test/T150-tagging.sh index 45471ac..4a2673d 100755 --- a/test/T150-tagging.sh +++ b/test/T150-tagging.sh @@ -261,4 +261,10 @@ test_expect_code 1 "Empty tag names" 'notmuch tag + One' test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One' +test_begin_subtest "Xapian exception: read only files" +chmod u-w ${MAIL_DIR}/.notmuch/xapian/*.DB +output=$(notmuch tag +something '*' 2>&1 | sed 's/: .*$//' ) +chmod u+w ${MAIL_DIR}/.notmuch/xapian/*.DB +test_expect_equal "$output" "A Xapian exception occurred opening database" + test_done -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 2/8] test: add support for compiling and running C snippets 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner 2015-03-24 13:24 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-25 16:09 ` Tomi Ollila 2015-03-24 13:24 ` [Patch v5 3/8] test: add error reporting tests David Bremner ` (5 subsequent siblings) 7 siblings, 1 reply; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch This is to limit the copy-pasta involved in running C tests. I decided to keep things simple and not try to provide an actual C skeleton. The setting of LD_LIBRARY_PATH is to force using the built libnotmuch rather than any potential system one. --- test/README | 5 +++++ test/test-lib.sh | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/test/README b/test/README index 81a1c82..5b40474 100644 --- a/test/README +++ b/test/README @@ -84,6 +84,11 @@ the tests in one of the following ways. TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient +Some tests may require a c compiler. You can choose the name and flags similarly +to with emacs, e.g. + + make test TEST_CC=gcc TEST_CFLAGS="-g -O2" + Quiet Execution --------------- diff --git a/test/test-lib.sh b/test/test-lib.sh index 133fbe4..fdb84ea 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -73,6 +73,8 @@ if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \ fi TEST_EMACS=${TEST_EMACS:-${EMACS:-emacs}} TEST_EMACSCLIENT=${TEST_EMACSCLIENT:-emacsclient} +TEST_CC=${TEST_CC:-cc} +TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"} # Protect ourselves from common misconfiguration to export # CDPATH into the environment @@ -1161,6 +1163,19 @@ test_python() { | $cmd - } +test_C () { + exec_file="test${test_count}" + test_file="${exec_file}.c" + cat > ${test_file} + export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib + ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch -ltalloc + echo "== stdout ==" > OUTPUT.stdout + echo "== stderr ==" > OUTPUT.stderr + ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr + sed "s,$(pwd),CWD," OUTPUT.stdout OUTPUT.stderr > OUTPUT +} + + # Creates a script that counts how much time it is executed and calls # notmuch. $notmuch_counter_command is set to the path to the # generated script. Use notmuch_counter_value() function to get the -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Patch v5 2/8] test: add support for compiling and running C snippets 2015-03-24 13:24 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner @ 2015-03-25 16:09 ` Tomi Ollila 0 siblings, 0 replies; 34+ messages in thread From: Tomi Ollila @ 2015-03-25 16:09 UTC (permalink / raw) To: David Bremner, David Bremner, notmuch On Tue, Mar 24 2015, David Bremner <david@tethera.net> wrote: > This is to limit the copy-pasta involved in running C tests. I decided > to keep things simple and not try to provide an actual C skeleton. > > The setting of LD_LIBRARY_PATH is to force using the built libnotmuch > rather than any potential system one. > --- > test/README | 5 +++++ > test/test-lib.sh | 15 +++++++++++++++ > 2 files changed, 20 insertions(+) > > diff --git a/test/README b/test/README > index 81a1c82..5b40474 100644 > --- a/test/README > +++ b/test/README > @@ -84,6 +84,11 @@ the tests in one of the following ways. > TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs > make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient > > +Some tests may require a c compiler. You can choose the name and flags similarly > +to with emacs, e.g. > + > + make test TEST_CC=gcc TEST_CFLAGS="-g -O2" > + > Quiet Execution > --------------- > > diff --git a/test/test-lib.sh b/test/test-lib.sh > index 133fbe4..fdb84ea 100644 > --- a/test/test-lib.sh > +++ b/test/test-lib.sh > @@ -73,6 +73,8 @@ if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \ > fi > TEST_EMACS=${TEST_EMACS:-${EMACS:-emacs}} > TEST_EMACSCLIENT=${TEST_EMACSCLIENT:-emacsclient} > +TEST_CC=${TEST_CC:-cc} > +TEST_CFLAGS=${TEST_CFLAGS:-"-g -O0"} > > # Protect ourselves from common misconfiguration to export > # CDPATH into the environment > @@ -1161,6 +1163,19 @@ test_python() { > | $cmd - > } > > +test_C () { > + exec_file="test${test_count}" > + test_file="${exec_file}.c" > + cat > ${test_file} > + export LD_LIBRARY_PATH=${TEST_DIRECTORY}/../lib > + ${TEST_CC} ${TEST_CFLAGS} -I${TEST_DIRECTORY}/../lib -o ${exec_file} ${test_file} -L${TEST_DIRECTORY}/../lib/ -lnotmuch -ltalloc > + echo "== stdout ==" > OUTPUT.stdout > + echo "== stderr ==" > OUTPUT.stderr > + ./${exec_file} "$@" 1>>OUTPUT.stdout 2>>OUTPUT.stderr > + sed "s,$(pwd),CWD," OUTPUT.stdout OUTPUT.stderr > OUTPUT > +} For next version you could change $(pwd) to ${PWD} -- to use shell variable instead of forking subshell and run builtin command there to get the same value. also, trailing g could be useful. i.e: sed "s,${PWD},CWD,g" OUTPUT.stdout OUTPUT.stderr > OUTPUT ^ permalink raw reply [flat|nested] 34+ messages in thread
* [Patch v5 3/8] test: add error reporting tests 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner 2015-03-24 13:24 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner 2015-03-24 13:24 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-25 16:19 ` Tomi Ollila 2015-03-24 13:24 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner ` (4 subsequent siblings) 7 siblings, 1 reply; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch This includes tests for all of the error fprintfs in the library I could figure out how to test without using gdb scripts. It boils down to errors opening files and exceptions caused by corrupted Xapian databases. --- test/T560-lib-error.sh | 250 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) create mode 100755 test/T560-lib-error.sh diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh new file mode 100755 index 0000000..c2964cb --- /dev/null +++ b/test/T560-lib-error.sh @@ -0,0 +1,250 @@ +#!/usr/bin/env bash +test_description="error reporting for library" + +. ./test-lib.sh + +backup_database (){ + rm -rf notmuch-dir-backup + cp -a ${MAIL_DIR}/.notmuch notmuch-dir-backup +} +restore_database (){ + rm -rf ${MAIL_DIR}/.notmuch + cp -a notmuch-dir-backup ${MAIL_DIR}/.notmuch +} + + +add_email_corpus + +test_expect_success "building database" "NOTMUCH_NEW" + +test_begin_subtest "Open null pointer" +test_C <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (NULL, 0, 0); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error: Cannot open a database for a NULL path. +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Open nonexistent database" +test_C <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open ("/nonexistent/foo", 0, 0); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error opening database at /nonexistent/foo/.notmuch: No such file or directory +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Write to read-only database" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_ONLY, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + stat = notmuch_database_add_message (db, "/dev/null", NULL); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Cannot write to a read-only database. +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Add non-existent file" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + stat = notmuch_database_add_message (db, "/nonexistent", NULL); + if (stat) + fputs (notmuch_database_status_string (db), stderr); + +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Error opening /nonexistent: No such file or directory +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "compact, overwriting existing backup" +test_C ${MAIL_DIR} <<EOF +#include <stdio.h> +#include <notmuch.h> +static void +status_cb (const char *msg, void *closure) +{ + printf ("%s\n", msg); +} +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_compact (argv[1], argv[1], status_cb, NULL); +} +EOF +cat <<EOF >EXPECTED +== stdout == +== stderr == +Path already exists: CWD/mail +EOF +test_expect_equal_file EXPECTED OUTPUT + +cat <<EOF > head.c +#include <stdio.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <talloc.h> +#include <notmuch.h> + +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + char *path; + int fd; + + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); + if (stat != NOTMUCH_STATUS_SUCCESS) { + fprintf (stderr, "error opening database: %d\n", stat); + } + path = talloc_asprintf (db, "%s/.notmuch/xapian/postlist.DB", argv[1]); + fd = open(path,O_WRONLY|O_TRUNC); + if (fd < 0) + fprintf (stderr, "error opening %s\n"); +EOF +cat <<EOF > tail.c + if (stat) { + const char *stat_str = notmuch_database_status_string (db); + if (stat_str) + fputs (stat_str, stderr); + } + +} +EOF + +backup_database +test_begin_subtest "Xapian exception finding message" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_message_t *message = NULL; + stat = notmuch_database_find_message (db, "id:nonexistant", &message); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred finding message +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception getting tags" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_tags_t *tags = NULL; + tags = notmuch_database_get_all_tags (db); + stat = (tags == NULL); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred getting tags +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception creating directory" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_directory_t *directory = NULL; + stat = notmuch_database_get_directory (db, "none/existing", &directory); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred creating a directory +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception searching messages" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_messages_t *messages = NULL; + notmuch_query_t *query=notmuch_query_create (db, "*"); + stat = notmuch_query_search_messages_st (query, &messages); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred performing query +Query string was: * +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +backup_database +test_begin_subtest "Xapian exception counting messages" +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} + { + notmuch_query_t *query=notmuch_query_create (db, "id:87ocn0qh6d.fsf@yoom.home.cworth.org"); + int count = notmuch_query_count_messages (query); + stat = (count == 0); + } +EOF +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean +cat <<EOF >EXPECTED +== stdout == +== stderr == +A Xapian exception occurred performing query +Query string was: id:87ocn0qh6d.fsf@yoom.home.cworth.org +EOF +test_expect_equal_file EXPECTED OUTPUT.clean +restore_database + +test_done -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Patch v5 3/8] test: add error reporting tests 2015-03-24 13:24 ` [Patch v5 3/8] test: add error reporting tests David Bremner @ 2015-03-25 16:19 ` Tomi Ollila 0 siblings, 0 replies; 34+ messages in thread From: Tomi Ollila @ 2015-03-25 16:19 UTC (permalink / raw) To: David Bremner, David Bremner, notmuch On Tue, Mar 24 2015, David Bremner <david@tethera.net> wrote: > This includes tests for all of the error fprintfs in the library I > could figure out how to test without using gdb scripts. It boils down > to errors opening files and exceptions caused by corrupted Xapian > databases. > --- > test/T560-lib-error.sh | 250 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 250 insertions(+) > create mode 100755 test/T560-lib-error.sh > > diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh > new file mode 100755 > index 0000000..c2964cb > --- /dev/null > +++ b/test/T560-lib-error.sh > @@ -0,0 +1,250 @@ > +#!/usr/bin/env bash > +test_description="error reporting for library" > + > +. ./test-lib.sh > + > +backup_database (){ missing space: backup_database () { ( fgrep '()' test/* shows consistent style there ) > + rm -rf notmuch-dir-backup > + cp -a ${MAIL_DIR}/.notmuch notmuch-dir-backup cp -a is not solaris compatible (it is freebsd 10.1) cp -pR would be more portable option > +} > +restore_database (){ missing space > + rm -rf ${MAIL_DIR}/.notmuch > + cp -a notmuch-dir-backup ${MAIL_DIR}/.notmuch cp -a ... > +} > + > + > +add_email_corpus > + > +test_expect_success "building database" "NOTMUCH_NEW" > + > +test_begin_subtest "Open null pointer" > +test_C <<EOF Like discussed in IRC, test_C <<'EOF' would be mode convenient to reviewer when there is nothing to be interpolated -- so reviewer can ignore checking potential interpolations (e.g. $... and `...`:s) grep '<<.*EOF' test/* doesn't show that this is ever user, though :/ > +#include <stdio.h> > +#include <notmuch.h> > +int main (int argc, char** argv) > +{ > + notmuch_database_t *db; > + notmuch_status_t stat; > + stat = notmuch_database_open (NULL, 0, 0); > +} > +EOF > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +Error: Cannot open a database for a NULL path. > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +test_begin_subtest "Open nonexistent database" > +test_C <<EOF > +#include <stdio.h> > +#include <notmuch.h> > +int main (int argc, char** argv) > +{ > + notmuch_database_t *db; > + notmuch_status_t stat; > + stat = notmuch_database_open ("/nonexistent/foo", 0, 0); > +} > +EOF > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +Error opening database at /nonexistent/foo/.notmuch: No such file or directory > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +test_begin_subtest "Write to read-only database" > +test_C ${MAIL_DIR} <<EOF > +#include <stdio.h> > +#include <notmuch.h> > +int main (int argc, char** argv) > +{ > + notmuch_database_t *db; > + notmuch_status_t stat; > + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_ONLY, &db); > + if (stat != NOTMUCH_STATUS_SUCCESS) { > + fprintf (stderr, "error opening database: %d\n", stat); > + } > + stat = notmuch_database_add_message (db, "/dev/null", NULL); > +} > +EOF > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +Cannot write to a read-only database. > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +test_begin_subtest "Add non-existent file" > +test_C ${MAIL_DIR} <<EOF > +#include <stdio.h> > +#include <notmuch.h> > +int main (int argc, char** argv) > +{ > + notmuch_database_t *db; > + notmuch_status_t stat; > + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); > + if (stat != NOTMUCH_STATUS_SUCCESS) { > + fprintf (stderr, "error opening database: %d\n", stat); > + } > + stat = notmuch_database_add_message (db, "/nonexistent", NULL); > + if (stat) > + fputs (notmuch_database_status_string (db), stderr); > + > +} > +EOF > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +Error opening /nonexistent: No such file or directory > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +test_begin_subtest "compact, overwriting existing backup" > +test_C ${MAIL_DIR} <<EOF > +#include <stdio.h> > +#include <notmuch.h> > +static void > +status_cb (const char *msg, void *closure) > +{ > + printf ("%s\n", msg); > +} > +int main (int argc, char** argv) > +{ > + notmuch_database_t *db; > + notmuch_status_t stat; > + stat = notmuch_database_compact (argv[1], argv[1], status_cb, NULL); > +} > +EOF > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +Path already exists: CWD/mail > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > +cat <<EOF > head.c One usually expects *.c files to be "complete" source files; as an alternative I suggest filenames c_head and c_tail > +#include <stdio.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <fcntl.h> > +#include <talloc.h> > +#include <notmuch.h> > + > +int main (int argc, char** argv) > +{ > + notmuch_database_t *db; > + notmuch_status_t stat; > + char *path; > + int fd; > + > + stat = notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db); > + if (stat != NOTMUCH_STATUS_SUCCESS) { > + fprintf (stderr, "error opening database: %d\n", stat); > + } > + path = talloc_asprintf (db, "%s/.notmuch/xapian/postlist.DB", argv[1]); > + fd = open(path,O_WRONLY|O_TRUNC); > + if (fd < 0) > + fprintf (stderr, "error opening %s\n"); > +EOF > +cat <<EOF > tail.c > + if (stat) { > + const char *stat_str = notmuch_database_status_string (db); > + if (stat_str) > + fputs (stat_str, stderr); > + } > + > +} > +EOF > + > +backup_database > +test_begin_subtest "Xapian exception finding message" > +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} > + { > + notmuch_message_t *message = NULL; > + stat = notmuch_database_find_message (db, "id:nonexistant", &message); > + } > +EOF > +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean (I was going to suggest sed 's/...' OUTPUT > OUTPUT.clean (i.e remove '<' but I actually cannot determine which would be better :D) > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +A Xapian exception occurred finding message > +EOF > +test_expect_equal_file EXPECTED OUTPUT.clean > +restore_database > + > +backup_database > +test_begin_subtest "Xapian exception getting tags" > +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} > + { > + notmuch_tags_t *tags = NULL; > + tags = notmuch_database_get_all_tags (db); > + stat = (tags == NULL); > + } > +EOF > +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +A Xapian exception occurred getting tags > +EOF > +test_expect_equal_file EXPECTED OUTPUT.clean > +restore_database > + > +backup_database > +test_begin_subtest "Xapian exception creating directory" > +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} > + { > + notmuch_directory_t *directory = NULL; > + stat = notmuch_database_get_directory (db, "none/existing", &directory); > + } > +EOF > +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +A Xapian exception occurred creating a directory > +EOF > +test_expect_equal_file EXPECTED OUTPUT.clean > +restore_database > + > +backup_database > +test_begin_subtest "Xapian exception searching messages" > +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} > + { > + notmuch_messages_t *messages = NULL; > + notmuch_query_t *query=notmuch_query_create (db, "*"); > + stat = notmuch_query_search_messages_st (query, &messages); > + } > +EOF > +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +A Xapian exception occurred performing query > +Query string was: * > +EOF > +test_expect_equal_file EXPECTED OUTPUT.clean > +restore_database > + > +backup_database > +test_begin_subtest "Xapian exception counting messages" > +cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} > + { > + notmuch_query_t *query=notmuch_query_create (db, "id:87ocn0qh6d.fsf@yoom.home.cworth.org"); > + int count = notmuch_query_count_messages (query); > + stat = (count == 0); > + } > +EOF > +sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean > +cat <<EOF >EXPECTED > +== stdout == > +== stderr == > +A Xapian exception occurred performing query > +Query string was: id:87ocn0qh6d.fsf@yoom.home.cworth.org > +EOF > +test_expect_equal_file EXPECTED OUTPUT.clean > +restore_database > + > +test_done > -- > 2.1.4 ^ permalink raw reply [flat|nested] 34+ messages in thread
* [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner ` (2 preceding siblings ...) 2015-03-24 13:24 ` [Patch v5 3/8] test: add error reporting tests David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-25 16:39 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open, create} Tomi Ollila 2015-03-24 13:24 ` [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t David Bremner ` (3 subsequent siblings) 7 siblings, 1 reply; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch The compatibility wrapper ensures that clients calling notmuch_database_open will receive consistent output for now. The changes to notmuch-{new,search} and test/symbol-test are just to make the test suite pass. The use of IGNORE_RESULT is justified by two things. 1) I don't know what else to do. 2) asprintf guarantees the output string is NULL if an error occurs, so at least we are not passing garbage back. --- lib/database.cc | 94 +++++++++++++++++++++++++++++++++++++---------------- lib/notmuch.h | 21 ++++++++++++ notmuch-new.c | 8 +++-- notmuch-search.c | 8 +++-- test/symbol-test.cc | 6 +++- 5 files changed, 104 insertions(+), 33 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 3974e2e..36849d7 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -608,29 +608,39 @@ 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"); + message = strdup ("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", - path, strerror (errno)); + IGNORE_RESULT (asprintf (&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", - path); + IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: " + "Not a directory.\n", + path)); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } @@ -640,15 +650,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", - notmuch_path, strerror (errno)); + IGNORE_RESULT (asprintf (&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, - ¬much); + status = notmuch_database_open_verbose (path, + NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, &message); if (status) goto DONE; @@ -667,6 +677,8 @@ notmuch_database_create (const char *path, notmuch_database_t **database) if (notmuch_path) talloc_free (notmuch_path); + if (message) + *status_string = message; if (database) *database = notmuch; else @@ -767,37 +779,55 @@ notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database) { + char *status_string = NULL; + notmuch_status_t status; + + status = notmuch_database_open_verbose(path, mode, database, + &status_string); + + if (status_string) fputs(status_string, stderr); + + return status; +} + +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, "Error: Cannot open a database for a NULL path.\n"); + message = strdup ("Error: Cannot open a database for a NULL path.\n"); status = NOTMUCH_STATUS_NULL_POINTER; goto DONE; } if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) { - fprintf (stderr, "Out of memory\n"); + message = strdup ("Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } err = stat (notmuch_path, &st); if (err) { - fprintf (stderr, "Error opening database at %s: %s\n", - notmuch_path, strerror (errno)); + IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n", + notmuch_path, strerror (errno))); status = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { - fprintf (stderr, "Out of memory\n"); + message = strdup ("Out of memory\n"); status = NOTMUCH_STATUS_OUT_OF_MEMORY; goto DONE; } @@ -837,11 +867,11 @@ notmuch_database_open (const char *path, * means a dramatically incompatible change. */ version = notmuch_database_get_version (notmuch); if (version > NOTMUCH_DATABASE_VERSION) { - fprintf (stderr, - "Error: Notmuch database at %s\n" - " has a newer database format version (%u) than supported by this\n" - " version of notmuch (%u).\n", - notmuch_path, version, NOTMUCH_DATABASE_VERSION); + IGNORE_RESULT (asprintf (&message, + "Error: Notmuch database at %s\n" + " has a newer database format version (%u) than supported by this\n" + " version of notmuch (%u).\n", + notmuch_path, version, NOTMUCH_DATABASE_VERSION)); notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; notmuch_database_destroy (notmuch); notmuch = NULL; @@ -856,11 +886,11 @@ notmuch_database_open (const char *path, version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r', &incompat_features); if (incompat_features) { - fprintf (stderr, - "Error: Notmuch database at %s\n" - " requires features (%s)\n" - " not supported by this version of notmuch.\n", - notmuch_path, incompat_features); + IGNORE_RESULT (asprintf (&message, + "Error: Notmuch database at %s\n" + " requires features (%s)\n" + " not supported by this version of notmuch.\n", + notmuch_path, incompat_features)); notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; notmuch_database_destroy (notmuch); notmuch = NULL; @@ -906,8 +936,8 @@ notmuch_database_open (const char *path, notmuch->query_parser->add_prefix (prefix->name, prefix->prefix); } } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred opening database: %s\n", - error.get_msg().c_str()); + IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n", + error.get_msg().c_str())); notmuch_database_destroy (notmuch); notmuch = NULL; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -916,6 +946,9 @@ notmuch_database_open (const char *path, DONE: talloc_free (local); + if (status_string && message) + *status_string = message; + if (database) *database = notmuch; else @@ -1039,13 +1072,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, ¬much); + ret = notmuch_database_open_verbose (path, + NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, + &message); if (ret) { + if (status_cb) status_cb (message, closure); goto DONE; } diff --git a/lib/notmuch.h b/lib/notmuch.h index f066245..c671d82 100644 --- a/lib/notmuch.h +++ b/lib/notmuch.h @@ -230,6 +230,16 @@ notmuch_status_t notmuch_database_create (const char *path, notmuch_database_t **database); /** + * Like notmuch_database_create, except optionally return an error + * message. This message is allocated by malloc and should be freed by + * the caller. + */ +notmuch_status_t +notmuch_database_create_verbose (const char *path, + notmuch_database_t **database, + char **error_message); + +/** * Database open mode for notmuch_database_open. */ typedef enum { @@ -279,6 +289,17 @@ notmuch_status_t notmuch_database_open (const char *path, notmuch_database_mode_t mode, notmuch_database_t **database); +/** + * Like notmuch_database_open, except optionally return an error + * message. This message is allocated by malloc and should be freed by + * the caller. + */ + +notmuch_status_t +notmuch_database_open_verbose (const char *path, + notmuch_database_mode_t mode, + notmuch_database_t **database, + char **error_message); /** * Commit changes and close the given notmuch database. diff --git a/notmuch-new.c b/notmuch-new.c index ddf42c1..93b70bf 100644 --- a/notmuch-new.c +++ b/notmuch-new.c @@ -985,9 +985,13 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) return EXIT_FAILURE; add_files_state.total_files = count; } else { - if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, - ¬much)) + char *status_string = NULL; + if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, + ¬much, &status_string)) { + if (status_string) fputs (status_string, stderr); + return EXIT_FAILURE; + } if (notmuch_database_needs_upgrade (notmuch)) { time_t now = time (NULL); diff --git a/notmuch-search.c b/notmuch-search.c index a591d45..d012af3 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar { char *query_str; unsigned int i; + char *status_string = NULL; switch (ctx->format_sel) { case NOTMUCH_FORMAT_TEXT: @@ -570,9 +571,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar notmuch_exit_if_unsupported_format (); - if (notmuch_database_open (notmuch_config_get_database_path (config), - NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch)) + if (notmuch_database_open_verbose ( + notmuch_config_get_database_path (config), + NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) { + if (status_string) fputs (status_string, stderr); return EXIT_FAILURE; + } query_str = query_string_from_args (ctx->notmuch, argc, argv); if (query_str == NULL) { diff --git a/test/symbol-test.cc b/test/symbol-test.cc index 3e96c03..9f8eea7 100644 --- a/test/symbol-test.cc +++ b/test/symbol-test.cc @@ -5,7 +5,11 @@ int main() { notmuch_database_t *notmuch; - notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much); + char *message = NULL; + + if (notmuch_database_open_verbose ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much, &message)) + if (message) fputs (message, stderr); + try { (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN); -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open, create} 2015-03-24 13:24 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner @ 2015-03-25 16:39 ` Tomi Ollila 2015-03-25 16:47 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner 0 siblings, 1 reply; 34+ messages in thread From: Tomi Ollila @ 2015-03-25 16:39 UTC (permalink / raw) To: David Bremner, David Bremner, notmuch On Tue, Mar 24 2015, David Bremner <david@tethera.net> wrote: > The compatibility wrapper ensures that clients calling > notmuch_database_open will receive consistent output for now. > > The changes to notmuch-{new,search} and test/symbol-test are just to > make the test suite pass. > > The use of IGNORE_RESULT is justified by two things. 1) I don't know > what else to do. 2) asprintf guarantees the output string is NULL if > an error occurs, so at least we are not passing garbage back. > --- > lib/database.cc | 94 +++++++++++++++++++++++++++++++++++++---------------- > lib/notmuch.h | 21 ++++++++++++ > notmuch-new.c | 8 +++-- > notmuch-search.c | 8 +++-- > test/symbol-test.cc | 6 +++- > 5 files changed, 104 insertions(+), 33 deletions(-) > > diff --git a/lib/database.cc b/lib/database.cc > index 3974e2e..36849d7 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -608,29 +608,39 @@ 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); Hmm, is is so that nothing is printed if creating database fails, should this provide &message to _verbose function and if message changed, fputs() it (and then free ())? ... after looking below -- notmuch_database_open () does it. > +} > + > +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"); > + message = strdup ("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", > - path, strerror (errno)); > + IGNORE_RESULT (asprintf (&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", > - path); > + IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: " > + "Not a directory.\n", > + path)); > status = NOTMUCH_STATUS_FILE_ERROR; > goto DONE; > } > @@ -640,15 +650,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", > - notmuch_path, strerror (errno)); > + IGNORE_RESULT (asprintf (&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, > - ¬much); > + status = notmuch_database_open_verbose (path, > + NOTMUCH_DATABASE_MODE_READ_WRITE, > + ¬much, &message); > if (status) > goto DONE; > > @@ -667,6 +677,8 @@ notmuch_database_create (const char *path, notmuch_database_t **database) > if (notmuch_path) > talloc_free (notmuch_path); > > + if (message) > + *status_string = message; Hmm, what if status_string == NULL -- do we have a test for this (or am I missing something ?) > if (database) > *database = notmuch; > else > @@ -767,37 +779,55 @@ notmuch_database_open (const char *path, > notmuch_database_mode_t mode, > notmuch_database_t **database) > { > + char *status_string = NULL; > + notmuch_status_t status; > + > + status = notmuch_database_open_verbose(path, mode, database, > + &status_string); > + > + if (status_string) fputs(status_string, stderr); and free (status_string); (and fputs (...) (i.e. space which is in all other hunks in this patch :) > + > + return status; > +} > + > +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, "Error: Cannot open a database for a NULL path.\n"); > + message = strdup ("Error: Cannot open a database for a NULL path.\n"); > status = NOTMUCH_STATUS_NULL_POINTER; > goto DONE; > } > > if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) { > - fprintf (stderr, "Out of memory\n"); > + message = strdup ("Out of memory\n"); > status = NOTMUCH_STATUS_OUT_OF_MEMORY; > goto DONE; > } > > err = stat (notmuch_path, &st); > if (err) { > - fprintf (stderr, "Error opening database at %s: %s\n", > - notmuch_path, strerror (errno)); > + IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n", > + notmuch_path, strerror (errno))); > status = NOTMUCH_STATUS_FILE_ERROR; > goto DONE; > } > > if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) { > - fprintf (stderr, "Out of memory\n"); > + message = strdup ("Out of memory\n"); > status = NOTMUCH_STATUS_OUT_OF_MEMORY; > goto DONE; > } > @@ -837,11 +867,11 @@ notmuch_database_open (const char *path, > * means a dramatically incompatible change. */ > version = notmuch_database_get_version (notmuch); > if (version > NOTMUCH_DATABASE_VERSION) { > - fprintf (stderr, > - "Error: Notmuch database at %s\n" > - " has a newer database format version (%u) than supported by this\n" > - " version of notmuch (%u).\n", > - notmuch_path, version, NOTMUCH_DATABASE_VERSION); > + IGNORE_RESULT (asprintf (&message, > + "Error: Notmuch database at %s\n" > + " has a newer database format version (%u) than supported by this\n" > + " version of notmuch (%u).\n", > + notmuch_path, version, NOTMUCH_DATABASE_VERSION)); > notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; > notmuch_database_destroy (notmuch); > notmuch = NULL; > @@ -856,11 +886,11 @@ notmuch_database_open (const char *path, > version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r', > &incompat_features); > if (incompat_features) { > - fprintf (stderr, > - "Error: Notmuch database at %s\n" > - " requires features (%s)\n" > - " not supported by this version of notmuch.\n", > - notmuch_path, incompat_features); > + IGNORE_RESULT (asprintf (&message, > + "Error: Notmuch database at %s\n" > + " requires features (%s)\n" > + " not supported by this version of notmuch.\n", > + notmuch_path, incompat_features)); > notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY; > notmuch_database_destroy (notmuch); > notmuch = NULL; > @@ -906,8 +936,8 @@ notmuch_database_open (const char *path, > notmuch->query_parser->add_prefix (prefix->name, prefix->prefix); > } > } catch (const Xapian::Error &error) { > - fprintf (stderr, "A Xapian exception occurred opening database: %s\n", > - error.get_msg().c_str()); > + IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n", > + error.get_msg().c_str())); > notmuch_database_destroy (notmuch); > notmuch = NULL; > status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; > @@ -916,6 +946,9 @@ notmuch_database_open (const char *path, > DONE: > talloc_free (local); > > + if (status_string && message) > + *status_string = message; here it is !!! \o/ !!! :D > + > if (database) > *database = notmuch; > else > @@ -1039,13 +1072,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, ¬much); > + ret = notmuch_database_open_verbose (path, > + NOTMUCH_DATABASE_MODE_READ_WRITE, > + ¬much, > + &message); > if (ret) { > + if (status_cb) status_cb (message, closure); > goto DONE; > } Is this ever printing the message (if any?) > > diff --git a/lib/notmuch.h b/lib/notmuch.h > index f066245..c671d82 100644 > --- a/lib/notmuch.h > +++ b/lib/notmuch.h > @@ -230,6 +230,16 @@ notmuch_status_t > notmuch_database_create (const char *path, notmuch_database_t **database); > > /** > + * Like notmuch_database_create, except optionally return an error > + * message. This message is allocated by malloc and should be freed by > + * the caller. > + */ > +notmuch_status_t > +notmuch_database_create_verbose (const char *path, > + notmuch_database_t **database, > + char **error_message); > + > +/** > * Database open mode for notmuch_database_open. > */ > typedef enum { > @@ -279,6 +289,17 @@ notmuch_status_t > notmuch_database_open (const char *path, > notmuch_database_mode_t mode, > notmuch_database_t **database); > +/** > + * Like notmuch_database_open, except optionally return an error > + * message. This message is allocated by malloc and should be freed by > + * the caller. > + */ > + > +notmuch_status_t > +notmuch_database_open_verbose (const char *path, > + notmuch_database_mode_t mode, > + notmuch_database_t **database, > + char **error_message); > > /** > * Commit changes and close the given notmuch database. > diff --git a/notmuch-new.c b/notmuch-new.c > index ddf42c1..93b70bf 100644 > --- a/notmuch-new.c > +++ b/notmuch-new.c > @@ -985,9 +985,13 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) > return EXIT_FAILURE; > add_files_state.total_files = count; > } else { > - if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, > - ¬much)) > + char *status_string = NULL; > + if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE, > + ¬much, &status_string)) { > + if (status_string) fputs (status_string, stderr); and free (status_string); > + > return EXIT_FAILURE; > + } > > if (notmuch_database_needs_upgrade (notmuch)) { > time_t now = time (NULL); > diff --git a/notmuch-search.c b/notmuch-search.c > index a591d45..d012af3 100644 > --- a/notmuch-search.c > +++ b/notmuch-search.c > @@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar > { > char *query_str; > unsigned int i; > + char *status_string = NULL; > > switch (ctx->format_sel) { > case NOTMUCH_FORMAT_TEXT: > @@ -570,9 +571,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar > > notmuch_exit_if_unsupported_format (); > > - if (notmuch_database_open (notmuch_config_get_database_path (config), > - NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch)) > + if (notmuch_database_open_verbose ( > + notmuch_config_get_database_path (config), > + NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) { > + if (status_string) fputs (status_string, stderr); diiitto (or do we skip freeing 'small' allocations just before exit) > return EXIT_FAILURE; > + } > > query_str = query_string_from_args (ctx->notmuch, argc, argv); > if (query_str == NULL) { > diff --git a/test/symbol-test.cc b/test/symbol-test.cc > index 3e96c03..9f8eea7 100644 > --- a/test/symbol-test.cc > +++ b/test/symbol-test.cc > @@ -5,7 +5,11 @@ > > int main() { > notmuch_database_t *notmuch; > - notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much); > + char *message = NULL; > + > + if (notmuch_database_open_verbose ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much, &message)) > + if (message) fputs (message, stderr); free... (for consistency) > + > > try { > (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN); > -- > 2.1.4 ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} 2015-03-25 16:39 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open, create} Tomi Ollila @ 2015-03-25 16:47 ` David Bremner 0 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-25 16:47 UTC (permalink / raw) To: Tomi Ollila, notmuch Tomi Ollila <tomi.ollila@iki.fi> writes: >> + ret = notmuch_database_open_verbose (path, >> + NOTMUCH_DATABASE_MODE_READ_WRITE, >> + ¬much, >> + &message); >> if (ret) { >> + if (status_cb) status_cb (message, closure); >> goto DONE; >> } > > Is this ever printing the message (if any?) Printing messages is the whole point of status_cb. It does switch the output from the current stderr to stdout, which is weird. On the other hand it avoids changing the api by adding notmuch_database_compact_verbose. And of course if we care this should free message after calling status_cb, like the other places. d ^ permalink raw reply [flat|nested] 34+ messages in thread
* [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner ` (3 preceding siblings ...) 2015-03-24 13:24 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-25 16:47 ` Tomi Ollila 2015-03-24 13:24 ` [Patch v5 6/8] lib: add private function to extract the database for a message David Bremner ` (2 subsequent siblings) 7 siblings, 1 reply; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch In principle in the future this could do something fancier than sprintf. --- lib/database-private.h | 4 ++++ lib/database.cc | 24 ++++++++++++++++++++++++ lib/notmuch-private.h | 4 ++++ lib/notmuch.h | 7 +++++++ 4 files changed, 39 insertions(+) diff --git a/lib/database-private.h b/lib/database-private.h index 6d6fa2c..24243db 100644 --- a/lib/database-private.h +++ b/lib/database-private.h @@ -154,6 +154,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 36849d7..673561b 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -348,6 +348,23 @@ notmuch_status_to_string (notmuch_status_t status) } } +void +_notmuch_database_log (notmuch_database_t *notmuch, + const char *format, + ...) +{ + va_list va_args; + + va_start (va_args, format); + + if (notmuch->status_string) + talloc_free (notmuch->status_string); + + notmuch->status_string = talloc_vasprintf (notmuch, format, va_args); + + va_end (va_args); +} + static void find_doc_ids_for_term (notmuch_database_t *notmuch, const char *term, @@ -845,6 +862,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] == '/') @@ -2530,3 +2548,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-private.h b/lib/notmuch-private.h index 8a1f2fa..7cb6fd4 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -190,6 +190,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); diff --git a/lib/notmuch.h b/lib/notmuch.h index c671d82..20c4e01 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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t 2015-03-24 13:24 ` [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t David Bremner @ 2015-03-25 16:47 ` Tomi Ollila 0 siblings, 0 replies; 34+ messages in thread From: Tomi Ollila @ 2015-03-25 16:47 UTC (permalink / raw) To: David Bremner, David Bremner, notmuch On Tue, Mar 24 2015, David Bremner <david@tethera.net> wrote: > In principle in the future this could do something fancier than sprintf. It would be better talking of sNprintf -- it is more accurate and potentially more educational. Rest of the patches in this series OK > --- > lib/database-private.h | 4 ++++ > lib/database.cc | 24 ++++++++++++++++++++++++ > lib/notmuch-private.h | 4 ++++ > lib/notmuch.h | 7 +++++++ > 4 files changed, 39 insertions(+) > > diff --git a/lib/database-private.h b/lib/database-private.h > index 6d6fa2c..24243db 100644 > --- a/lib/database-private.h > +++ b/lib/database-private.h > @@ -154,6 +154,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 36849d7..673561b 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -348,6 +348,23 @@ notmuch_status_to_string (notmuch_status_t status) > } > } > > +void > +_notmuch_database_log (notmuch_database_t *notmuch, > + const char *format, > + ...) > +{ > + va_list va_args; > + > + va_start (va_args, format); > + > + if (notmuch->status_string) > + talloc_free (notmuch->status_string); > + > + notmuch->status_string = talloc_vasprintf (notmuch, format, va_args); > + > + va_end (va_args); > +} > + > static void > find_doc_ids_for_term (notmuch_database_t *notmuch, > const char *term, > @@ -845,6 +862,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] == '/') > @@ -2530,3 +2548,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-private.h b/lib/notmuch-private.h > index 8a1f2fa..7cb6fd4 100644 > --- a/lib/notmuch-private.h > +++ b/lib/notmuch-private.h > @@ -190,6 +190,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); > diff --git a/lib/notmuch.h b/lib/notmuch.h > index c671d82..20c4e01 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.4 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 34+ messages in thread
* [Patch v5 6/8] lib: add private function to extract the database for a message. 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner ` (4 preceding siblings ...) 2015-03-24 13:24 ` [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-24 13:24 ` [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log David Bremner 2015-03-24 13:24 ` [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open David Bremner 7 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch 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 956a70a..b84e5e1 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -1626,3 +1626,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 7cb6fd4..dc58a2f 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -472,6 +472,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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner ` (5 preceding siblings ...) 2015-03-24 13:24 ` [Patch v5 6/8] lib: add private function to extract the database for a message David Bremner @ 2015-03-24 13:24 ` David Bremner 2015-03-24 13:24 ` [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open David Bremner 7 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch 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 | 38 +++++++++++++++++++++----------------- lib/directory.cc | 4 ++-- lib/index.cc | 11 +++++++---- lib/message.cc | 6 +++--- lib/query.cc | 18 ++++++++++++------ test/T560-lib-error.sh | 6 +++++- 6 files changed, 50 insertions(+), 33 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 673561b..155b5aa 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -473,7 +473,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; @@ -707,7 +707,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; } @@ -1000,7 +1000,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()); } } @@ -1132,12 +1132,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; @@ -1157,20 +1157,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; @@ -1178,7 +1178,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; @@ -1189,6 +1189,10 @@ notmuch_database_compact (const char *path, if (notmuch) { notmuch_status_t ret2; + const char *str = notmuch_database_status_string (notmuch); + if (status_cb && str) + status_cb (str, closure); + ret2 = notmuch_database_destroy (notmuch); /* don't clobber previous error status */ @@ -1207,7 +1211,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 @@ -1485,7 +1489,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; @@ -1535,7 +1539,7 @@ notmuch_database_begin_atomic (notmuch_database_t *notmuch) try { (static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db))->begin_transaction (false); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred beginning transaction: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred beginning transaction: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -1569,7 +1573,7 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch) if (thresh && atoi (thresh) == 1) db->flush (); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred committing transaction: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred committing transaction: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -1815,7 +1819,7 @@ notmuch_database_get_directory (notmuch_database_t *notmuch, *directory = _notmuch_directory_create (notmuch, path, NOTMUCH_FIND_LOOKUP, &status); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred getting directory: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2397,7 +2401,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch, _notmuch_message_sync (message); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred adding message: %s.\n", + _notmuch_database_log (notmuch, "A Xapian exception occurred adding message: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; ret = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2489,7 +2493,7 @@ notmuch_database_find_message_by_filename (notmuch_database_t *notmuch, status = NOTMUCH_STATUS_OUT_OF_MEMORY; } } catch (const Xapian::Error &error) { - fprintf (stderr, "Error: A Xapian exception occurred finding message by filename: %s\n", + _notmuch_database_log (notmuch, "Error: A Xapian exception occurred finding message by filename: %s\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; status = NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -2542,7 +2546,7 @@ notmuch_database_get_all_tags (notmuch_database_t *db) _notmuch_string_list_sort (tags); return _notmuch_tags_create (db, tags); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred getting tags: %s.\n", + _notmuch_database_log (db, "A Xapian exception occurred getting tags: %s.\n", error.get_msg().c_str()); db->exception_reported = TRUE; return NULL; diff --git a/lib/directory.cc b/lib/directory.cc index 8daaec8..b836ea2 100644 --- a/lib/directory.cc +++ b/lib/directory.cc @@ -186,7 +186,7 @@ _notmuch_directory_create (notmuch_database_t *notmuch, directory->mtime = Xapian::sortable_unserialise ( directory->doc.get_value (NOTMUCH_VALUE_TIMESTAMP)); } catch (const Xapian::Error &error) { - fprintf (stderr, + _notmuch_database_log (notmuch, "A Xapian exception occurred creating a directory: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; @@ -228,7 +228,7 @@ notmuch_directory_set_mtime (notmuch_directory_t *directory, db->replace_document (directory->document_id, directory->doc); } catch (const Xapian::Error &error) { - fprintf (stderr, + _notmuch_database_log (notmuch, "A Xapian exception occurred setting directory mtime: %s.\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; diff --git a/lib/index.cc b/lib/index.cc index c88ed8d..e81aa81 100644 --- a/lib/index.cc +++ b/lib/index.cc @@ -314,7 +314,8 @@ _index_mime_part (notmuch_message_t *message, const char *charset; if (! part) { - fprintf (stderr, "Warning: Not indexing empty mime part.\n"); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Not indexing empty mime part.\n"); return; } @@ -344,7 +345,8 @@ _index_mime_part (notmuch_message_t *message, if (i == 1) continue; if (i > 1) - fprintf (stderr, "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n"); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Unexpected extra parts of multipart/signed. Indexing anyway.\n"); } if (GMIME_IS_MULTIPART_ENCRYPTED (multipart)) { /* Don't index encrypted parts. */ @@ -367,8 +369,9 @@ _index_mime_part (notmuch_message_t *message, } if (! (GMIME_IS_PART (part))) { - fprintf (stderr, "Warning: Not indexing unknown mime part: %s.\n", - g_type_name (G_OBJECT_TYPE (part))); + _notmuch_database_log (_notmuch_message_database (message), + "Warning: Not indexing unknown mime part: %s.\n", + g_type_name (G_OBJECT_TYPE (part))); return; } diff --git a/lib/message.cc b/lib/message.cc index b84e5e1..a8ca988 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -252,7 +252,7 @@ _notmuch_message_create_for_message_id (notmuch_database_t *notmuch, doc_id = _notmuch_database_generate_doc_id (notmuch); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred creating message: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred creating message: %s\n", error.get_msg().c_str()); notmuch->exception_reported = TRUE; *status_ret = NOTMUCH_PRIVATE_STATUS_XAPIAN_EXCEPTION; @@ -467,7 +467,7 @@ notmuch_message_get_header (notmuch_message_t *message, const char *header) return talloc_strdup (message, value.c_str ()); } catch (Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred when reading header: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading header: %s\n", error.get_msg().c_str()); message->notmuch->exception_reported = TRUE; return NULL; @@ -921,7 +921,7 @@ notmuch_message_get_date (notmuch_message_t *message) try { value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP); } catch (Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred when reading date: %s\n", + _notmuch_database_log(_notmuch_message_database (message), "A Xapian exception occurred when reading date: %s\n", error.get_msg().c_str()); message->notmuch->exception_reported = TRUE; return 0; diff --git a/lib/query.cc b/lib/query.cc index 57aa6d2..9cedb6a 100644 --- a/lib/query.cc +++ b/lib/query.cc @@ -296,9 +296,12 @@ notmuch_query_search_messages_st (notmuch_query_t *query, return NOTMUCH_STATUS_SUCCESS; } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred performing query: %s\n", - error.get_msg().c_str()); - fprintf (stderr, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, + "A Xapian exception occurred performing query: %s\n" + "Query string was: %s\n", + error.get_msg().c_str(), + query->query_string); + notmuch->exception_reported = TRUE; talloc_free (messages); return NOTMUCH_STATUS_XAPIAN_EXCEPTION; @@ -597,9 +600,12 @@ notmuch_query_count_messages (notmuch_query_t *query) count = mset.get_matches_estimated(); } catch (const Xapian::Error &error) { - fprintf (stderr, "A Xapian exception occurred: %s\n", - error.get_msg().c_str()); - fprintf (stderr, "Query string was: %s\n", query->query_string); + _notmuch_database_log (notmuch, + "A Xapian exception occurred performing query: %s\n" + "Query string was: %s\n", + error.get_msg().c_str(), + query->query_string); + } return count; diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh index c2964cb..ec7552a 100755 --- a/test/T560-lib-error.sh +++ b/test/T560-lib-error.sh @@ -66,6 +66,9 @@ int main (int argc, char** argv) fprintf (stderr, "error opening database: %d\n", stat); } stat = notmuch_database_add_message (db, "/dev/null", NULL); + if (stat) + fputs (notmuch_database_status_string (db), stderr); + } EOF cat <<EOF >EXPECTED @@ -118,8 +121,9 @@ int main (int argc, char** argv) EOF cat <<EOF >EXPECTED == stdout == -== stderr == Path already exists: CWD/mail + +== stderr == EOF test_expect_equal_file EXPECTED OUTPUT -- 2.1.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open 2015-03-24 13:24 ` Update to library logging, version 5 David Bremner ` (6 preceding siblings ...) 2015-03-24 13:24 ` [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log David Bremner @ 2015-03-24 13:24 ` David Bremner 7 siblings, 0 replies; 34+ messages in thread From: David Bremner @ 2015-03-24 13:24 UTC (permalink / raw) To: David Bremner, notmuch 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. --- 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 155b5aa..85054df 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -2292,7 +2292,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 a41d9ad..8ac96e8 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 a8ca988..5bc7aff 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 dc58a2f..cc9ce12 100644 --- a/lib/notmuch-private.h +++ b/lib/notmuch-private.h @@ -358,11 +358,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.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
end of thread, other threads:[~2015-03-25 16:48 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-14 17:02 libnotmuch logging overhaul v4 David Bremner
2015-03-14 17:02 ` [Patch v4 1/9] test: Add two tests for error output from notmuch_database_open David Bremner
2015-03-14 17:02 ` [Patch v4 2/9] test: add support for compiling and running C snippets David Bremner
2015-03-21 8:51 ` Tomi Ollila
2015-03-14 17:02 ` [Patch v4 3/9] test: add error reporting tests for lib/database.cc David Bremner
2015-03-14 17:02 ` [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner
2015-03-21 9:27 ` [Patch v4 4/9] lib: add "verbose" versions of notmuch_database_{open, create} Tomi Ollila
2015-03-14 17:02 ` [Patch v4 5/9] lib/database: add field for last error string David Bremner
2015-03-14 17:02 ` [Patch v4 6/9] lib: add a log function with output to a string in notmuch_database_t David Bremner
2015-03-14 17:02 ` [Patch v4 7/9] lib: add private function to extract the database for a message David Bremner
2015-03-14 17:02 ` [Patch v4 8/9] lib: replace almost all fprintfs in library with _n_d_log David Bremner
2015-03-14 17:02 ` [Patch v4 9/9] lib: eliminate fprintf from _notmuch_message_file_open David Bremner
2015-03-24 13:19 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner
2015-03-24 13:19 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner
2015-03-24 13:19 ` [Patch v5 3/8] test: add error reporting tests David Bremner
2015-03-24 13:19 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner
2015-03-24 13:19 ` [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t David Bremner
2015-03-24 13:19 ` [Patch v5 6/8] lib: add private function to extract the database for a message David Bremner
2015-03-24 13:19 ` [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log David Bremner
2015-03-24 13:19 ` [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open David Bremner
2015-03-24 13:24 ` Update to library logging, version 5 David Bremner
2015-03-24 13:24 ` [Patch v5 1/8] test: Add two tests for error output from notmuch_database_open David Bremner
2015-03-24 13:24 ` [Patch v5 2/8] test: add support for compiling and running C snippets David Bremner
2015-03-25 16:09 ` Tomi Ollila
2015-03-24 13:24 ` [Patch v5 3/8] test: add error reporting tests David Bremner
2015-03-25 16:19 ` Tomi Ollila
2015-03-24 13:24 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner
2015-03-25 16:39 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open, create} Tomi Ollila
2015-03-25 16:47 ` [Patch v5 4/8] lib: add "verbose" versions of notmuch_database_{open,create} David Bremner
2015-03-24 13:24 ` [Patch v5 5/8] lib: add a log function with output to a string in notmuch_database_t David Bremner
2015-03-25 16:47 ` Tomi Ollila
2015-03-24 13:24 ` [Patch v5 6/8] lib: add private function to extract the database for a message David Bremner
2015-03-24 13:24 ` [Patch v5 7/8] lib: replace almost all fprintfs in library with _n_d_log David Bremner
2015-03-24 13:24 ` [Patch v5 8/8] lib: eliminate fprintf from _notmuch_message_file_open David Bremner
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).