From 699a976cf552aeb3c5f4f44d51905b4edf41c7b4 Mon Sep 17 00:00:00 2001 From: David Bremner Date: Wed, 25 Mar 2015 09:24:03 +2000 Subject: [PATCH] Update to library logging, version 5 --- ed/7b3a3ca7a1ea55eacb40a62d66ac7e0d1ca727 | 489 ++++++++++++++++++++++ 1 file changed, 489 insertions(+) create mode 100644 ed/7b3a3ca7a1ea55eacb40a62d66ac7e0d1ca727 diff --git a/ed/7b3a3ca7a1ea55eacb40a62d66ac7e0d1ca727 b/ed/7b3a3ca7a1ea55eacb40a62d66ac7e0d1ca727 new file mode 100644 index 000000000..4280f0c57 --- /dev/null +++ b/ed/7b3a3ca7a1ea55eacb40a62d66ac7e0d1ca727 @@ -0,0 +1,489 @@ +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 + } + + -- 2.26.2