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 A2ECA431FB6 for ; Fri, 27 Mar 2015 15:12:45 -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 2r6VSRbBs5Yk for ; Fri, 27 Mar 2015 15:12:42 -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 0DDCD431FAF for ; Fri, 27 Mar 2015 15:12:41 -0700 (PDT) Received: from remotemail by mx.xen14.node3324.gplhost.com with local (Exim 4.80) (envelope-from ) id 1YbcUI-0000EI-AB for notmuch@notmuchmail.org; Fri, 27 Mar 2015 22:12:22 +0000 Received: (nullmailer pid 1795 invoked by uid 1000); Fri, 27 Mar 2015 22:12:09 -0000 From: David Bremner To: notmuch@notmuchmail.org Subject: Library logging overhaul, round 6 Date: Fri, 27 Mar 2015 23:11:52 +0100 Message-Id: <1427494320-1483-1-git-send-email-david@tethera.net> X-Mailer: git-send-email 2.1.4 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: Fri, 27 Mar 2015 22:12:46 -0000 This obsoletes id:1427203451-1540-1-git-send-email-david@tethera.net I think this addresses all of Tomi's comments, except the use of status_cb to print error output from notmuch_database_compact. I added some tests for notmuch_database_create error output. diff --git a/lib/database.cc b/lib/database.cc index 85054df..9f66b5f 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -625,7 +625,18 @@ 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); + char *status_string = NULL; + notmuch_status_t status; + + status = notmuch_database_create_verbose (path, database, + &status_string); + + if (status_string) { + fputs (status_string, stderr); + free (status_string); + } + + return status; } notmuch_status_t @@ -694,8 +705,9 @@ notmuch_database_create_verbose (const char *path, if (notmuch_path) talloc_free (notmuch_path); - if (message) + if (status_string && message) *status_string = message; + if (database) *database = notmuch; else @@ -799,10 +811,13 @@ notmuch_database_open (const char *path, char *status_string = NULL; notmuch_status_t status; - status = notmuch_database_open_verbose(path, mode, database, + status = notmuch_database_open_verbose (path, mode, database, &status_string); - if (status_string) fputs(status_string, stderr); + if (status_string) { + fputs (status_string, stderr); + free (status_string); + } return status; } diff --git a/notmuch-new.c b/notmuch-new.c index 93b70bf..e6c283e 100644 --- a/notmuch-new.c +++ b/notmuch-new.c @@ -988,7 +988,10 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[]) 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); + if (status_string) { + fputs (status_string, stderr); + free (status_string); + } return EXIT_FAILURE; } diff --git a/notmuch-search.c b/notmuch-search.c index d012af3..b81ac01 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -574,7 +574,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar 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); + + if (status_string) { + fputs (status_string, stderr); + free (status_string); + } + return EXIT_FAILURE; } diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh index ec7552a..67a5e8d 100755 --- a/test/T560-lib-error.sh +++ b/test/T560-lib-error.sh @@ -3,13 +3,13 @@ test_description="error reporting for library" . ./test-lib.sh -backup_database (){ +backup_database () { rm -rf notmuch-dir-backup - cp -a ${MAIL_DIR}/.notmuch notmuch-dir-backup + cp -pR ${MAIL_DIR}/.notmuch notmuch-dir-backup } -restore_database (){ +restore_database () { rm -rf ${MAIL_DIR}/.notmuch - cp -a notmuch-dir-backup ${MAIL_DIR}/.notmuch + cp -pR notmuch-dir-backup ${MAIL_DIR}/.notmuch } @@ -18,7 +18,7 @@ add_email_corpus test_expect_success "building database" "NOTMUCH_NEW" test_begin_subtest "Open null pointer" -test_C < #include int main (int argc, char** argv) @@ -28,7 +28,7 @@ int main (int argc, char** argv) stat = notmuch_database_open (NULL, 0, 0); } EOF -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == Error: Cannot open a database for a NULL path. @@ -36,7 +36,7 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest "Open nonexistent database" -test_C < #include int main (int argc, char** argv) @@ -46,15 +46,50 @@ int main (int argc, char** argv) stat = notmuch_database_open ("/nonexistent/foo", 0, 0); } EOF -cat <EXPECTED +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 "create NULL path" +test_C <<'EOF' +#include +#include +int main (int argc, char** argv) +{ + notmuch_status_t stat; + stat = notmuch_database_create (NULL, NULL); +} +EOF +cat <<'EOF' >EXPECTED +== stdout == +== stderr == +Error: Cannot create a database for a NULL path. +EOF +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Create database in non-existant directory" +test_C <<'EOF' +#include +#include +int main (int argc, char** argv) +{ + notmuch_database_t *db; + notmuch_status_t stat; + stat = notmuch_database_create ("/nonexistent/foo", &db); +} +EOF +cat <<'EOF' >EXPECTED +== stdout == +== stderr == +Error: Cannot create database at /nonexistent/foo: No such file or directory. +EOF +test_expect_equal_file EXPECTED OUTPUT + test_begin_subtest "Write to read-only database" -test_C ${MAIL_DIR} < #include int main (int argc, char** argv) @@ -71,7 +106,7 @@ int main (int argc, char** argv) } EOF -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == Cannot write to a read-only database. @@ -79,7 +114,7 @@ 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) @@ -96,7 +131,7 @@ int main (int argc, char** argv) } EOF -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == Error opening /nonexistent: No such file or directory @@ -104,7 +139,7 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest "compact, overwriting existing backup" -test_C ${MAIL_DIR} < #include static void @@ -119,7 +154,7 @@ int main (int argc, char** argv) stat = notmuch_database_compact (argv[1], argv[1], status_cb, NULL); } EOF -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == Path already exists: CWD/mail @@ -127,7 +162,7 @@ Path already exists: CWD/mail EOF test_expect_equal_file EXPECTED OUTPUT -cat < head.c +cat <<'EOF' > c_head #include #include #include @@ -151,7 +186,7 @@ int main (int argc, char** argv) if (fd < 0) fprintf (stderr, "error opening %s\n"); EOF -cat < tail.c +cat <<'EOF' > c_tail if (stat) { const char *stat_str = notmuch_database_status_string (db); if (stat_str) @@ -163,14 +198,14 @@ EOF backup_database test_begin_subtest "Xapian exception finding message" -cat head.c - tail.c < OUTPUT.clean -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == A Xapian exception occurred finding message @@ -180,7 +215,7 @@ restore_database backup_database test_begin_subtest "Xapian exception getting tags" -cat head.c - tail.c < OUTPUT.clean -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == A Xapian exception occurred getting tags @@ -198,14 +233,14 @@ restore_database backup_database test_begin_subtest "Xapian exception creating directory" -cat head.c - tail.c < OUTPUT.clean -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == A Xapian exception occurred creating a directory @@ -215,7 +250,7 @@ restore_database backup_database test_begin_subtest "Xapian exception searching messages" -cat head.c - tail.c < OUTPUT.clean -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == A Xapian exception occurred performing query @@ -234,7 +269,7 @@ restore_database backup_database test_begin_subtest "Xapian exception counting messages" -cat head.c - tail.c < OUTPUT.clean -cat <EXPECTED +cat <<'EOF' >EXPECTED == stdout == == stderr == A Xapian exception occurred performing query diff --git a/test/symbol-test.cc b/test/symbol-test.cc index 9f8eea7..d979f83 100644 --- a/test/symbol-test.cc +++ b/test/symbol-test.cc @@ -1,4 +1,5 @@ #include +#include #include #include @@ -8,8 +9,10 @@ int main() { char *message = NULL; if (notmuch_database_open_verbose ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much, &message)) - if (message) fputs (message, stderr); - + if (message) { + fputs (message, stderr); + free (message); + } try { (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN); diff --git a/test/test-lib.sh b/test/test-lib.sh index fdb84ea..486d1c4 100644 --- a/test/test-lib.sh +++ b/test/test-lib.sh @@ -1172,7 +1172,7 @@ test_C () { 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 + sed "s,${PWD},CWD,g" OUTPUT.stdout OUTPUT.stderr > OUTPUT }