Return-Path: X-Original-To: notmuch@notmuchmail.org Delivered-To: notmuch@notmuchmail.org Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id C6889431E62 for ; Tue, 24 Mar 2015 06:25:14 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: 2.438 X-Spam-Level: ** X-Spam-Status: No, score=2.438 tagged_above=-999 required=5 tests=[DNS_FROM_AHBL_RHSBL=2.438] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F+XJB1qPsWwq for ; Tue, 24 Mar 2015 06:25:11 -0700 (PDT) Received: from mx.xen14.node3324.gplhost.com (gitolite.debian.net [87.98.215.224]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 26C44431FD2 for ; Tue, 24 Mar 2015 06:25:11 -0700 (PDT) Received: from remotemail by mx.xen14.node3324.gplhost.com with local (Exim 4.80) (envelope-from ) id 1YaOon-0000rq-Pd; Tue, 24 Mar 2015 13:24:29 +0000 Received: (nullmailer pid 2634 invoked by uid 1000); Tue, 24 Mar 2015 13:24:19 -0000 From: David Bremner To: David Bremner , notmuch@notmuchmail.org Subject: Update to library logging, version 5 Date: Tue, 24 Mar 2015 09:24:03 -0400 Message-Id: <1427203451-1540-1-git-send-email-david@tethera.net> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1426352554-4383-10-git-send-email-david@tethera.net> References: <1426352554-4383-10-git-send-email-david@tethera.net> X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Mar 2015 13:25:15 -0000 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} < +#include +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 <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} < @@ -92,4 +127,128 @@ Path already exists: CWD/mail EOF test_expect_equal_file EXPECTED OUTPUT +cat < head.c +#include +#include +#include +#include +#include +#include + +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 < 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 < OUTPUT.clean +cat <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 < OUTPUT.clean +cat <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 < OUTPUT.clean +cat <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 < OUTPUT.clean +cat <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 < OUTPUT.clean +cat <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 }