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 <<EOF +test_C <<'EOF' #include <stdio.h> #include <notmuch.h> 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 <<EOF >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 <<EOF +test_C <<'EOF' #include <stdio.h> #include <notmuch.h> 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 <<EOF >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 <stdio.h> +#include <notmuch.h> +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 <stdio.h> +#include <notmuch.h> +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} <<EOF +test_C ${MAIL_DIR} <<'EOF' #include <stdio.h> #include <notmuch.h> int main (int argc, char** argv) @@ -71,7 +106,7 @@ int main (int argc, char** argv) } EOF -cat <<EOF >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} <<EOF +test_C ${MAIL_DIR} <<'EOF' #include <stdio.h> #include <notmuch.h> int main (int argc, char** argv) @@ -96,7 +131,7 @@ int main (int argc, char** argv) } EOF -cat <<EOF >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} <<EOF +test_C ${MAIL_DIR} <<'EOF' #include <stdio.h> #include <notmuch.h> 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 <<EOF >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 <<EOF > head.c +cat <<'EOF' > c_head #include <stdio.h> #include <sys/types.h> #include <sys/stat.h> @@ -151,7 +186,7 @@ int main (int argc, char** argv) if (fd < 0) fprintf (stderr, "error opening %s\n"); EOF -cat <<EOF > 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 <<EOF | test_C ${MAIL_DIR} +cat c_head - c_tail <<'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 +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 <<EOF | test_C ${MAIL_DIR} +cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} { notmuch_tags_t *tags = NULL; tags = notmuch_database_get_all_tags (db); @@ -188,7 +223,7 @@ cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} } EOF sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean -cat <<EOF >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 <<EOF | test_C ${MAIL_DIR} +cat c_head - c_tail <<'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 +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 <<EOF | test_C ${MAIL_DIR} +cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} { notmuch_messages_t *messages = NULL; notmuch_query_t *query=notmuch_query_create (db, "*"); @@ -223,7 +258,7 @@ cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} } EOF sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean -cat <<EOF >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 <<EOF | test_C ${MAIL_DIR} +cat c_head - c_tail <<'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); @@ -242,7 +277,7 @@ cat head.c - tail.c <<EOF | test_C ${MAIL_DIR} } EOF sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean -cat <<EOF >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 <stdio.h> +#include <stdlib.h> #include <xapian.h> #include <notmuch.h> @@ -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 }