From a9a9e374e2567caad8601d1781a3b0af8a3dde13 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Thu, 19 Jan 2012 17:29:19 -0500
Subject: Silence buildbot warnings about unused results

This ignores the results of the two writes in sigint handlers even
harder than before.

While my libc lacks the declarations that trigger these warnings, this
can be tested by adding the following to notmuch.h:

__attribute__((warn_unused_result))
ssize_t write(int fd, const void *buf, size_t count);
---
 notmuch-new.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index 3512de7..a569a54 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -67,7 +67,11 @@ handle_sigint (unused (int sig))
 {
     static char msg[] = "Stopping...         \n";
 
-    (void) write(2, msg, sizeof(msg)-1);
+    /* This write is "opportunistic", so it's okay to ignore the
+     * result.  It is not required for correctness, and if it does
+     * fail or produce a short write, we want to get out of the signal
+     * handler as quickly as possible, not retry it. */
+    IGNORE_RESULT (write (2, msg, sizeof(msg)-1));
     interrupted = 1;
 }
 
-- 
cgit v1.2.3


From 5f39979a4aa2aea7d587201702fa3443248334bd Mon Sep 17 00:00:00 2001
From: Ethan Glasser-Camp <ethan@betacantrips.com>
Date: Tue, 7 Feb 2012 05:05:03 -0500
Subject: Free the results of scandir()

scandir() returns "strings allocated via malloc(3)" which are then
"collected in array namelist which is allocated via
malloc(3)". Currently we just free the array namelist. Instead, free
all the entries of namelist, and then free namelist.

entry only points to elements of namelist, so we don't free it
separately.
---
 notmuch-new.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index a569a54..8dbebb3 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -559,12 +559,14 @@ add_files_recursive (notmuch_database_t *notmuch,
   DONE:
     if (next)
 	talloc_free (next);
-    if (entry)
-	free (entry);
     if (dir)
 	closedir (dir);
-    if (fs_entries)
+    if (fs_entries) {
+	for (i = 0; i < num_fs_entries; i++)
+	    free (fs_entries[i]);
+
 	free (fs_entries);
+    }
     if (db_subdirs)
 	notmuch_filenames_destroy (db_subdirs);
     if (db_files)
@@ -704,10 +706,12 @@ count_files (const char *path, int *count)
     }
 
   DONE:
-    if (entry)
-	free (entry);
-    if (fs_entries)
+    if (fs_entries) {
+	for (i = 0; i < num_fs_entries; i++)
+	    free (fs_entries[i]);
+
         free (fs_entries);
+    }
 }
 
 static void
-- 
cgit v1.2.3


From ce1e720de64270a7cbb4bc3fba2c7fe081de3edc Mon Sep 17 00:00:00 2001
From: Tomi Ollila <tomi.ollila@iki.fi>
Date: Wed, 15 Feb 2012 11:17:31 +0200
Subject: add support for user-specified files & directories to ignore

A new configuration key 'new.ignore' is used to determine which
files and directories user wants not to be scanned as new mails.

Mark the corresponding test as no longer broken.
This work merges my previous attempts and Andreas Amann's work
in id:"ylp7hi23mw8.fsf@tyndall.ie"
---
 notmuch-client.h |  9 +++++++++
 notmuch-config.c | 30 +++++++++++++++++++++++++++++-
 notmuch-new.c    | 45 +++++++++++++++++++++++++++++++++------------
 test/new         |  1 -
 4 files changed, 71 insertions(+), 14 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-client.h b/notmuch-client.h
index 60828aa..f4a62cc 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -250,6 +250,15 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
 			     const char *new_tags[],
 			     size_t length);
 
+const char **
+notmuch_config_get_new_ignore (notmuch_config_t *config,
+			       size_t *length);
+
+void
+notmuch_config_set_new_ignore (notmuch_config_t *config,
+			       const char *new_ignore[],
+			       size_t length);
+
 notmuch_bool_t
 notmuch_config_get_maildir_synchronize_flags (notmuch_config_t *config);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index a124e34..1f01128 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -44,7 +44,10 @@ static const char new_config_comment[] =
     " The following options are supported here:\n"
     "\n"
     "\ttags	A list (separated by ';') of the tags that will be\n"
-    "\t	added to all messages incorporated by \"notmuch new\".\n";
+    "\t	added to all messages incorporated by \"notmuch new\".\n"
+    "\n"
+    "\tignore	A list (separated by ';') of file and directory names\n"
+    "\t	that will not be searched for messages by \"notmuch new\".\n";
 
 static const char user_config_comment[] =
     " User configuration\n"
@@ -105,6 +108,8 @@ struct _notmuch_config {
     size_t user_other_email_length;
     const char **new_tags;
     size_t new_tags_length;
+    const char **new_ignore;
+    size_t new_ignore_length;
     notmuch_bool_t maildir_synchronize_flags;
     const char **search_exclude_tags;
     size_t search_exclude_tags_length;
@@ -264,6 +269,8 @@ notmuch_config_open (void *ctx,
     config->user_other_email_length = 0;
     config->new_tags = NULL;
     config->new_tags_length = 0;
+    config->new_ignore = NULL;
+    config->new_ignore_length = 0;
     config->maildir_synchronize_flags = TRUE;
     config->search_exclude_tags = NULL;
     config->search_exclude_tags_length = 0;
@@ -361,6 +368,10 @@ notmuch_config_open (void *ctx,
 	notmuch_config_set_new_tags (config, tags, 2);
     }
 
+    if (notmuch_config_get_new_ignore (config, &tmp) == NULL) {
+	notmuch_config_set_new_ignore (config, NULL, 0);
+    }
+
     if (notmuch_config_get_search_exclude_tags (config, &tmp) == NULL) {
 	if (is_new) {
 	    const char *tags[] = { "deleted", "spam" };
@@ -609,6 +620,14 @@ notmuch_config_get_new_tags (notmuch_config_t *config,   size_t *length)
 			     &(config->new_tags_length), length);
 }
 
+const char **
+notmuch_config_get_new_ignore (notmuch_config_t *config, size_t *length)
+{
+    return _config_get_list (config, "new", "ignore",
+			     &(config->new_ignore),
+			     &(config->new_ignore_length), length);
+}
+
 void
 notmuch_config_set_user_other_email (notmuch_config_t *config,
 				     const char *list[],
@@ -627,6 +646,15 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
 		     &(config->new_tags));
 }
 
+void
+notmuch_config_set_new_ignore (notmuch_config_t *config,
+			       const char *list[],
+			       size_t length)
+{
+    _config_set_list (config, "new", "ignore", list, length,
+		     &(config->new_ignore));
+}
+
 const char **
 notmuch_config_get_search_exclude_tags (notmuch_config_t *config, size_t *length)
 {
diff --git a/notmuch-new.c b/notmuch-new.c
index 8dbebb3..4f13535 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -39,6 +39,8 @@ typedef struct {
     int verbose;
     const char **new_tags;
     size_t new_tags_length;
+    const char **new_ignore;
+    size_t new_ignore_length;
 
     int total_files;
     int processed_files;
@@ -181,6 +183,20 @@ _entries_resemble_maildir (struct dirent **entries, int count)
     return 0;
 }
 
+/* Test if the file/directory is to be ignored.
+ */
+static notmuch_bool_t
+_entry_in_ignore_list (const char *entry, add_files_state_t *state)
+{
+    size_t i;
+
+    for (i = 0; i < state->new_ignore_length; i++)
+	if (strcmp (entry, state->new_ignore[i]) == 0)
+	    return TRUE;
+
+    return FALSE;
+}
+
 /* Examine 'path' recursively as follows:
  *
  *   o Ask the filesystem for the mtime of 'path' (fs_mtime)
@@ -320,15 +336,15 @@ add_files_recursive (notmuch_database_t *notmuch,
 	}
 
 	/* Ignore special directories to avoid infinite recursion.
-	 * Also ignore the .notmuch directory and any "tmp" directory
-	 * that appears within a maildir.
+	 * Also ignore the .notmuch directory, any "tmp" directory
+	 * that appears within a maildir and files/directories
+	 * the user has configured to be ignored.
 	 */
-	/* XXX: Eventually we'll want more sophistication to let the
-	 * user specify files to be ignored. */
 	if (strcmp (entry->d_name, ".") == 0 ||
 	    strcmp (entry->d_name, "..") == 0 ||
 	    (is_maildir && strcmp (entry->d_name, "tmp") == 0) ||
-	    strcmp (entry->d_name, ".notmuch") ==0)
+	    strcmp (entry->d_name, ".notmuch") == 0 ||
+	    _entry_in_ignore_list (entry->d_name, state))
 	{
 	    continue;
 	}
@@ -369,6 +385,10 @@ add_files_recursive (notmuch_database_t *notmuch,
 
         entry = fs_entries[i];
 
+	/* Ignore files & directories user has configured to be ignored */
+	if (_entry_in_ignore_list (entry->d_name, state))
+	    continue;
+
 	/* Check if we've walked past any names in db_files or
 	 * db_subdirs. If so, these have been deleted. */
 	while (notmuch_filenames_valid (db_files) &&
@@ -650,7 +670,7 @@ add_files (notmuch_database_t *notmuch,
  * initialized to zero by the top-level caller before calling
  * count_files). */
 static void
-count_files (const char *path, int *count)
+count_files (const char *path, int *count, add_files_state_t *state)
 {
     struct dirent *entry = NULL;
     char *next;
@@ -672,13 +692,13 @@ count_files (const char *path, int *count)
         entry = fs_entries[i++];
 
 	/* Ignore special directories to avoid infinite recursion.
-	 * Also ignore the .notmuch directory.
+	 * Also ignore the .notmuch directory and files/directories
+	 * the user has configured to be ignored.
 	 */
-	/* XXX: Eventually we'll want more sophistication to let the
-	 * user specify files to be ignored. */
 	if (strcmp (entry->d_name, ".") == 0 ||
 	    strcmp (entry->d_name, "..") == 0 ||
-	    strcmp (entry->d_name, ".notmuch") == 0)
+	    strcmp (entry->d_name, ".notmuch") == 0 ||
+	    _entry_in_ignore_list (entry->d_name, state))
 	{
 	    continue;
 	}
@@ -699,7 +719,7 @@ count_files (const char *path, int *count)
 		fflush (stdout);
 	    }
 	} else if (S_ISDIR (st.st_mode)) {
-	    count_files (next, count);
+	    count_files (next, count, state);
 	}
 
 	free (next);
@@ -841,6 +861,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	return 1;
 
     add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
+    add_files_state.new_ignore = notmuch_config_get_new_ignore (config, &add_files_state.new_ignore_length);
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
     db_path = notmuch_config_get_database_path (config);
 
@@ -856,7 +877,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	int count;
 
 	count = 0;
-	count_files (db_path, &count);
+	count_files (db_path, &count, &add_files_state);
 	if (interrupted)
 	    return 1;
 
diff --git a/test/new b/test/new
index e453684..79a6c97 100755
--- a/test/new
+++ b/test/new
@@ -167,7 +167,6 @@ Note: Ignoring non-mail file: ${MAIL_DIR}/ignored_file
 Added 1 new message to the database."
 
 test_begin_subtest "Ignore files and directories specified in new.ignore"
-test_subtest_known_broken
 generate_message
 notmuch config set new.ignore .git ignored_file .ignored_hidden_file
 touch "${MAIL_DIR}"/.git # change .git's mtime for notmuch new to rescan.
-- 
cgit v1.2.3


From e075ee37c9d42673e7e8b1d045915841b059735c Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Sun, 22 Apr 2012 11:50:49 -0400
Subject: new: Consistently treat fatal errors as fatal

Previously, fatal errors in add_files_recursive were not treated as
fatal by its callers (including itself!).  This makes
add_files_recursive errors consistently fatal and updates all callers
to treat them as fatal.
---
 notmuch-new.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index 4f13535..15c0b36 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -308,6 +308,10 @@ add_files_recursive (notmuch_database_t *notmuch,
     if (num_fs_entries == -1) {
 	fprintf (stderr, "Error opening directory %s: %s\n",
 		 path, strerror (errno));
+	/* We consider this a fatal error because, if a user moved a
+	 * message from another directory that we were able to scan
+	 * into this directory, skipping this directory will cause
+	 * that message to be lost. */
 	ret = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
@@ -351,8 +355,10 @@ add_files_recursive (notmuch_database_t *notmuch,
 
 	next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
 	status = add_files_recursive (notmuch, next, state);
-	if (status && ret == NOTMUCH_STATUS_SUCCESS)
+	if (status) {
 	    ret = status;
+	    goto DONE;
+	}
 	talloc_free (next);
 	next = NULL;
     }
@@ -933,6 +939,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     }
 
     ret = add_files (notmuch, db_path, &add_files_state);
+    if (ret)
+	goto DONE;
 
     gettimeofday (&tv_start, NULL);
     for (f = add_files_state.removed_files->head; f && !interrupted; f = f->next) {
@@ -965,6 +973,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	}
     }
 
+  DONE:
     talloc_free (add_files_state.removed_files);
     talloc_free (add_files_state.removed_directories);
     talloc_free (add_files_state.directory_mtimes);
@@ -1012,10 +1021,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     printf ("\n");
 
-    if (ret) {
-	printf ("\nNote: At least one error was encountered: %s\n",
+    if (ret)
+	printf ("\nNote: A fatal error was encountered: %s\n",
 		notmuch_status_to_string (ret));
-    }
 
     notmuch_database_close (notmuch);
 
-- 
cgit v1.2.3


From d3b5533123293fdc1e4177f42018f085c03585c9 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Sun, 22 Apr 2012 11:50:50 -0400
Subject: new: Handle fatal errors in remove_filename and _remove_directory

Previously such errors were simply ignored.  Now they cause an
immediate cleanup and abort.
---
 notmuch-new.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index 15c0b36..2faf2f8 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -785,8 +785,10 @@ remove_filename (notmuch_database_t *notmuch,
 	add_files_state->renamed_messages++;
 	if (add_files_state->synchronize_flags == TRUE)
 	    notmuch_message_maildir_flags_to_tags (message);
-    } else
+	status = NOTMUCH_STATUS_SUCCESS;
+    } else if (status == NOTMUCH_STATUS_SUCCESS) {
 	add_files_state->removed_messages++;
+    }
     notmuch_message_destroy (message);
     notmuch_database_end_atomic (notmuch);
     return status;
@@ -794,12 +796,13 @@ remove_filename (notmuch_database_t *notmuch,
 
 /* Recursively remove all filenames from the database referring to
  * 'path' (or to any of its children). */
-static void
+static notmuch_status_t
 _remove_directory (void *ctx,
 		   notmuch_database_t *notmuch,
 		   const char *path,
 		   add_files_state_t *add_files_state)
 {
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     notmuch_directory_t *directory;
     notmuch_filenames_t *files, *subdirs;
     char *absolute;
@@ -812,8 +815,10 @@ _remove_directory (void *ctx,
     {
 	absolute = talloc_asprintf (ctx, "%s/%s", path,
 				    notmuch_filenames_get (files));
-	remove_filename (notmuch, absolute, add_files_state);
+	status = remove_filename (notmuch, absolute, add_files_state);
 	talloc_free (absolute);
+	if (status)
+	    goto DONE;
     }
 
     for (subdirs = notmuch_directory_get_child_directories (directory);
@@ -822,11 +827,15 @@ _remove_directory (void *ctx,
     {
 	absolute = talloc_asprintf (ctx, "%s/%s", path,
 				    notmuch_filenames_get (subdirs));
-	_remove_directory (ctx, notmuch, absolute, add_files_state);
+	status = _remove_directory (ctx, notmuch, absolute, add_files_state);
 	talloc_free (absolute);
+	if (status)
+	    goto DONE;
     }
 
+  DONE:
     notmuch_directory_destroy (directory);
+    return status;
 }
 
 int
@@ -944,7 +953,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     gettimeofday (&tv_start, NULL);
     for (f = add_files_state.removed_files->head; f && !interrupted; f = f->next) {
-	remove_filename (notmuch, f->filename, &add_files_state);
+	ret = remove_filename (notmuch, f->filename, &add_files_state);
+	if (ret)
+	    goto DONE;
 	if (do_print_progress) {
 	    do_print_progress = 0;
 	    generic_print_progress ("Cleaned up", "messages",
@@ -955,7 +966,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     gettimeofday (&tv_start, NULL);
     for (f = add_files_state.removed_directories->head, i = 0; f && !interrupted; f = f->next, i++) {
-	_remove_directory (ctx, notmuch, f->filename, &add_files_state);
+	ret = _remove_directory (ctx, notmuch, f->filename, &add_files_state);
+	if (ret)
+	    goto DONE;
 	if (do_print_progress) {
 	    do_print_progress = 0;
 	    generic_print_progress ("Cleaned up", "directories",
-- 
cgit v1.2.3


From 746fef0aeafe1f29720140ab8778cdee22d519cb Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Sun, 22 Apr 2012 11:50:51 -0400
Subject: new: Print final fatal error message to stderr

This was going to stdout.  I removed the newline at the beginning of
printing the fatal error message because it wouldn't make sense if you
were only looking at the stderr stream (e.g., you had redirected
stdout to /dev/null).
---
 notmuch-new.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index 2faf2f8..bf9b120 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1035,8 +1035,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     printf ("\n");
 
     if (ret)
-	printf ("\nNote: A fatal error was encountered: %s\n",
-		notmuch_status_to_string (ret));
+	fprintf (stderr, "Note: A fatal error was encountered: %s\n",
+		 notmuch_status_to_string (ret));
 
     notmuch_database_close (notmuch);
 
-- 
cgit v1.2.3


From 2e7b6494046342872f1f79418679b1554d6d1005 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Sun, 22 Apr 2012 11:50:52 -0400
Subject: new: Fix missing end_atomic in remove_filename on error

Previously, if we failed to find the message by filename in
remove_filename, we would return immediately from the function without
ending its atomic block.  Now this code follows the usual goto DONE
idiom to perform cleanup.
---
 notmuch-new.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index bf9b120..473201e 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -779,7 +779,8 @@ remove_filename (notmuch_database_t *notmuch,
 	return status;
     status = notmuch_database_find_message_by_filename (notmuch, path, &message);
     if (status || message == NULL)
-	return status;
+	goto DONE;
+
     status = notmuch_database_remove_message (notmuch, path);
     if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
 	add_files_state->renamed_messages++;
@@ -790,6 +791,8 @@ remove_filename (notmuch_database_t *notmuch,
 	add_files_state->removed_messages++;
     }
     notmuch_message_destroy (message);
+
+  DONE:
     notmuch_database_end_atomic (notmuch);
     return status;
 }
-- 
cgit v1.2.3


From 6f7469f54744656f90ce215f365d5731e16acd3c Mon Sep 17 00:00:00 2001
From: Justus Winter <4winter@informatik.uni-hamburg.de>
Date: Sun, 22 Apr 2012 14:07:53 +0200
Subject: Use notmuch_database_destroy instead of notmuch_database_close

Adapt the notmuch binaries source to the notmuch_database_close split.

Signed-off-by: Justus Winter <4winter@informatik.uni-hamburg.de>
---
 notmuch-count.c   | 2 +-
 notmuch-dump.c    | 2 +-
 notmuch-new.c     | 2 +-
 notmuch-reply.c   | 2 +-
 notmuch-restore.c | 2 +-
 notmuch-search.c  | 2 +-
 notmuch-show.c    | 2 +-
 notmuch-tag.c     | 2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-count.c b/notmuch-count.c
index b76690c..9c2ad7b 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -107,7 +107,7 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
     }
 
     notmuch_query_destroy (query);
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
 
     return 0;
 }
diff --git a/notmuch-dump.c b/notmuch-dump.c
index a735875..71ab0ea 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -116,7 +116,7 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
 	fclose (output);
 
     notmuch_query_destroy (query);
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
 
     return 0;
 }
diff --git a/notmuch-new.c b/notmuch-new.c
index 473201e..3ff6304 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1041,7 +1041,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	fprintf (stderr, "Note: A fatal error was encountered: %s\n",
 		 notmuch_status_to_string (ret));
 
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
 
     if (run_hooks && !ret && !interrupted)
 	ret = notmuch_run_hook (db_path, "post-new");
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 0949d9f..da99a13 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -755,7 +755,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	return 1;
 
     notmuch_query_destroy (query);
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
 
     if (params.cryptoctx)
 	g_object_unref(params.cryptoctx);
diff --git a/notmuch-restore.c b/notmuch-restore.c
index d3b9246..02b563c 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -192,7 +192,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     if (line)
 	free (line);
 
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
     if (input != stdin)
 	fclose (input);
 
diff --git a/notmuch-search.c b/notmuch-search.c
index 1cc8430..7dfd270 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -545,7 +545,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     }
 
     notmuch_query_destroy (query);
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
 
     return ret;
 }
diff --git a/notmuch-show.c b/notmuch-show.c
index da4a797..3b6667c 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1117,7 +1117,7 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     }
 
     notmuch_query_destroy (query);
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
 
     if (params.cryptoctx)
 	g_object_unref(params.cryptoctx);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 05feed3..bd56fd1 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -238,7 +238,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 
     ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags);
 
-    notmuch_database_close (notmuch);
+    notmuch_database_destroy (notmuch);
 
     return ret;
 }
-- 
cgit v1.2.3


From 5fddc07dc31481453c1af186bf7da241c00cdbf1 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Mon, 30 Apr 2012 12:25:33 -0400
Subject: lib/cli: Make notmuch_database_open return a status code

It has been a long-standing issue that notmuch_database_open doesn't
return any indication of why it failed.  This patch changes its
prototype to return a notmuch_status_t and set an out-argument to the
database itself, like other functions that return both a status and an
object.

In the interest of atomicity, this also updates every use in the CLI
so that notmuch still compiles.  Since this patch does not update the
bindings, the Python bindings test fails.
---
 lib/database.cc     | 28 +++++++++++++++++++++++-----
 lib/notmuch.h       | 26 +++++++++++++++++++-------
 notmuch-count.c     |  5 ++---
 notmuch-dump.c      |  5 ++---
 notmuch-new.c       |  5 ++---
 notmuch-reply.c     |  5 ++---
 notmuch-restore.c   |  5 ++---
 notmuch-search.c    |  5 ++---
 notmuch-show.c      |  5 ++---
 notmuch-tag.c       |  5 ++---
 test/symbol-test.cc |  3 ++-
 11 files changed, 60 insertions(+), 37 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/lib/database.cc b/lib/database.cc
index 2fefcad..1e66599 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -556,8 +556,9 @@ notmuch_database_create (const char *path)
 	goto DONE;
     }
 
-    notmuch = notmuch_database_open (path,
-				     NOTMUCH_DATABASE_MODE_READ_WRITE);
+    notmuch_database_open (path,
+			   NOTMUCH_DATABASE_MODE_READ_WRITE,
+			   &notmuch);
     notmuch_database_upgrade (notmuch, NULL, NULL);
 
   DONE:
@@ -578,10 +579,12 @@ _notmuch_database_ensure_writable (notmuch_database_t *notmuch)
     return NOTMUCH_STATUS_SUCCESS;
 }
 
-notmuch_database_t *
+notmuch_status_t
 notmuch_database_open (const char *path,
-		       notmuch_database_mode_t mode)
+		       notmuch_database_mode_t mode,
+		       notmuch_database_t **database)
 {
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path, *xapian_path;
@@ -590,8 +593,15 @@ notmuch_database_open (const char *path,
     unsigned int i, version;
     static int initialized = 0;
 
+    if (path == NULL) {
+	fprintf (stderr, "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");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
 
@@ -599,11 +609,13 @@ notmuch_database_open (const char *path,
     if (err) {
 	fprintf (stderr, "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");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
 
@@ -644,6 +656,7 @@ notmuch_database_open (const char *path,
 		notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
 		notmuch_database_destroy (notmuch);
 		notmuch = NULL;
+		status = NOTMUCH_STATUS_FILE_ERROR;
 		goto DONE;
 	    }
 
@@ -704,12 +717,17 @@ notmuch_database_open (const char *path,
 		 error.get_msg().c_str());
 	notmuch_database_destroy (notmuch);
 	notmuch = NULL;
+	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
 
   DONE:
     talloc_free (local);
 
-    return notmuch;
+    if (database)
+	*database = notmuch;
+    else
+	talloc_free (notmuch);
+    return status;
 }
 
 void
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 7d9e092..44b0c46 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -151,9 +151,6 @@ typedef enum {
     NOTMUCH_DATABASE_MODE_READ_WRITE
 } notmuch_database_mode_t;
 
-/* XXX: I think I'd like this to take an extra argument of
- * notmuch_status_t* for returning a status value on failure. */
-
 /* Open an existing notmuch database located at 'path'.
  *
  * The database should have been created at some time in the past,
@@ -168,12 +165,27 @@ typedef enum {
  * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
- * In case of any failure, this function returns NULL, (after printing
- * an error message on stderr).
+ * In case of any failure, this function returns an error status and
+ * sets *database to NULL (after printing an error message on stderr).
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully opened the database.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'path' argument is NULL.
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.
+ *
+ * NOTMUCH_STATUS_FILE_ERROR: An error occurred trying to open the
+ *	database file (such as permission denied, or file not found,
+ *	etc.), or the database version is unknown.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
  */
-notmuch_database_t *
+notmuch_status_t
 notmuch_database_open (const char *path,
-		       notmuch_database_mode_t mode);
+		       notmuch_database_mode_t mode,
+		       notmuch_database_t **database);
 
 /* Close the given notmuch database.
  *
diff --git a/notmuch-count.c b/notmuch-count.c
index 9c2ad7b..2f98128 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -66,9 +66,8 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query_str = query_string_from_args (ctx, argc-opt_index, argv+opt_index);
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 71ab0ea..3743214 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -36,9 +36,8 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     char *output_file_name = NULL;
diff --git a/notmuch-new.c b/notmuch-new.c
index 3ff6304..7788743 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -903,9 +903,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	notmuch = notmuch_database_create (db_path);
 	add_files_state.total_files = count;
     } else {
-	notmuch = notmuch_database_open (db_path,
-					 NOTMUCH_DATABASE_MODE_READ_WRITE);
-	if (notmuch == NULL)
+	if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
+				   &notmuch))
 	    return 1;
 
 	if (notmuch_database_needs_upgrade (notmuch)) {
diff --git a/notmuch-reply.c b/notmuch-reply.c
index da99a13..7184a5d 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -740,9 +740,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query = notmuch_query_create (notmuch, query_string);
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 02b563c..4f4096e 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -115,9 +115,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_WRITE);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
 	return 1;
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
diff --git a/notmuch-search.c b/notmuch-search.c
index 7dfd270..3be296d 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -486,9 +486,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index);
diff --git a/notmuch-show.c b/notmuch-show.c
index 3b6667c..95427d4 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1081,9 +1081,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_ONLY);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch))
 	return 1;
 
     query = notmuch_query_create (notmuch, query_string);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index bd56fd1..7d18639 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -229,9 +229,8 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    notmuch = notmuch_database_open (notmuch_config_get_database_path (config),
-				     NOTMUCH_DATABASE_MODE_READ_WRITE);
-    if (notmuch == NULL)
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
 	return 1;
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
diff --git a/test/symbol-test.cc b/test/symbol-test.cc
index 1548ca4..3e96c03 100644
--- a/test/symbol-test.cc
+++ b/test/symbol-test.cc
@@ -4,7 +4,8 @@
 
 
 int main() {
-  (void) notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY);
+  notmuch_database_t *notmuch;
+  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);
 
   try {
     (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
-- 
cgit v1.2.3


From ba5729421825e0ec9d38aa9d656553f329aa6f09 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Mon, 30 Apr 2012 12:25:34 -0400
Subject: lib/cli: Make notmuch_database_create return a status code

This is the notmuch_database_create equivalent of the previous change.

In this case, there were places where errors were not being propagated
correctly in notmuch_database_create or in calls to it.  These have
been fixed, using the new status value.
---
 lib/database.cc | 29 ++++++++++++++++++++++-------
 lib/notmuch.h   | 22 ++++++++++++++++++----
 notmuch-new.c   |  3 ++-
 3 files changed, 42 insertions(+), 12 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/lib/database.cc b/lib/database.cc
index 1e66599..07f186e 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -520,9 +520,10 @@ parse_references (void *ctx,
     }
 }
 
-notmuch_database_t *
-notmuch_database_create (const char *path)
+notmuch_status_t
+notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path = NULL;
     struct stat st;
@@ -530,6 +531,7 @@ notmuch_database_create (const char *path)
 
     if (path == NULL) {
 	fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");
+	status = NOTMUCH_STATUS_NULL_POINTER;
 	goto DONE;
     }
 
@@ -537,12 +539,14 @@ notmuch_database_create (const char *path)
     if (err) {
 	fprintf (stderr, "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);
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
@@ -553,19 +557,30 @@ notmuch_database_create (const char *path)
     if (err) {
 	fprintf (stderr, "Error: Cannot create directory %s: %s.\n",
 		 notmuch_path, strerror (errno));
+	status = NOTMUCH_STATUS_FILE_ERROR;
 	goto DONE;
     }
 
-    notmuch_database_open (path,
-			   NOTMUCH_DATABASE_MODE_READ_WRITE,
-			   &notmuch);
-    notmuch_database_upgrade (notmuch, NULL, NULL);
+    status = notmuch_database_open (path,
+				    NOTMUCH_DATABASE_MODE_READ_WRITE,
+				    &notmuch);
+    if (status)
+	goto DONE;
+    status = notmuch_database_upgrade (notmuch, NULL, NULL);
+    if (status) {
+	notmuch_database_close(notmuch);
+	notmuch = NULL;
+    }
 
   DONE:
     if (notmuch_path)
 	talloc_free (notmuch_path);
 
-    return notmuch;
+    if (database)
+	*database = notmuch;
+    else
+	talloc_free (notmuch);
+    return status;
 }
 
 notmuch_status_t
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 44b0c46..e6e5cc2 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -140,11 +140,25 @@ typedef struct _notmuch_filenames notmuch_filenames_t;
  * contained within 'path' can be added to the database by calling
  * notmuch_database_add_message.
  *
- * In case of any failure, this function returns NULL, (after printing
- * an error message on stderr).
+ * In case of any failure, this function returns an error status and
+ * sets *database to NULL (after printing an error message on stderr).
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully created the database.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'path' argument is NULL.
+ *
+ * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.
+ *
+ * NOTMUCH_STATUS_FILE_ERROR: An error occurred trying to create the
+ *	database file (such as permission denied, or file not found,
+ *	etc.), or the database already exists.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
  */
-notmuch_database_t *
-notmuch_database_create (const char *path);
+notmuch_status_t
+notmuch_database_create (const char *path, notmuch_database_t **database);
 
 typedef enum {
     NOTMUCH_DATABASE_MODE_READ_ONLY = 0,
diff --git a/notmuch-new.c b/notmuch-new.c
index 7788743..cb720cc 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -900,7 +900,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 	    return 1;
 
 	printf ("Found %d total files (that's not much mail).\n", count);
-	notmuch = notmuch_database_create (db_path);
+	if (notmuch_database_create (db_path, &notmuch))
+	    return 1;
 	add_files_state.total_files = count;
     } else {
 	if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
-- 
cgit v1.2.3


From 7199d22f4394abdf72ab791fc0aba2c697bf1209 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Sun, 13 May 2012 19:36:09 -0400
Subject: lib/cli: Make notmuch_database_get_directory return a status code

Previously, notmuch_database_get_directory had no way to indicate how
it had failed.  This changes its prototype to return a status code and
set an out-argument to the retrieved directory, like similar functions
in the library API.  This does *not* change its currently broken
behavior of creating directory objects when they don't exist, but it
does document it and paves the way for fixing this.  Also, it can now
check for a read-only database and return
NOTMUCH_STATUS_READ_ONLY_DATABASE instead of crashing.

In the interest of atomicity, this also updates calls from the CLI so
that notmuch still compiles.
---
 lib/database.cc | 22 ++++++++++++++++------
 lib/notmuch.h   | 23 ++++++++++++++++++++---
 notmuch-new.c   | 21 ++++++++++++++-------
 3 files changed, 50 insertions(+), 16 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/lib/database.cc b/lib/database.cc
index 07f186e..f8c4a7d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -955,8 +955,8 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
 		mtime = Xapian::sortable_unserialise (
 		    document.get_value (NOTMUCH_VALUE_TIMESTAMP));
 
-		directory = notmuch_database_get_directory (notmuch,
-							    term.c_str() + 10);
+		directory = _notmuch_directory_create (notmuch, term.c_str() + 10,
+						       &status);
 		notmuch_directory_set_mtime (directory, mtime);
 		notmuch_directory_destroy (directory);
 	    }
@@ -1304,20 +1304,30 @@ _notmuch_database_relative_path (notmuch_database_t *notmuch,
     return relative;
 }
 
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *notmuch,
-				const char *path)
+				const char *path,
+				notmuch_directory_t **directory)
 {
     notmuch_status_t status;
 
+    if (directory == NULL)
+	return NOTMUCH_STATUS_NULL_POINTER;
+    *directory = NULL;
+
+    /* XXX Handle read-only databases properly */
+    if (notmuch->mode == NOTMUCH_DATABASE_MODE_READ_ONLY)
+	return NOTMUCH_STATUS_READ_ONLY_DATABASE;
+
     try {
-	return _notmuch_directory_create (notmuch, path, &status);
+	*directory = _notmuch_directory_create (notmuch, path, &status);
     } catch (const Xapian::Error &error) {
 	fprintf (stderr, "A Xapian exception occurred getting directory: %s.\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
-	return NULL;
+	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
+    return status;
 }
 
 /* Allocate a document ID that satisfies the following criteria:
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e6e5cc2..bbb17e4 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -300,11 +300,28 @@ notmuch_database_end_atomic (notmuch_database_t *notmuch);
  * (see notmuch_database_get_path), or else should be an absolute path
  * with initial components that match the path of 'database'.
  *
- * Can return NULL if a Xapian exception occurs.
+ * Note: Currently this will create the directory object if it doesn't
+ * exist.  In the future, when a directory object does not exist this
+ * will return NOTMUCH_STATUS_SUCCESS and set *directory to NULL.
+ * Callers should be prepared for this.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: Successfully retrieved directory.
+ *
+ * NOTMUCH_STATUS_NULL_POINTER: The given 'directory' argument is NULL.
+ *
+ * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred;
+ *	directory not retrieved.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ *	mode so the directory cannot be created (this case will be
+ *	removed in the future).
  */
-notmuch_directory_t *
+notmuch_status_t
 notmuch_database_get_directory (notmuch_database_t *database,
-				const char *path);
+				const char *path,
+				notmuch_directory_t **directory);
 
 /* Add a new message to the given notmuch database or associate an
  * additional filename with an existing message.
diff --git a/notmuch-new.c b/notmuch-new.c
index cb720cc..a3a8bec 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -250,7 +250,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
     notmuch_message_t *message = NULL;
     struct dirent **fs_entries = NULL;
-    int i, num_fs_entries;
+    int i, num_fs_entries = 0;
     notmuch_directory_t *directory;
     notmuch_filenames_t *db_files = NULL;
     notmuch_filenames_t *db_subdirs = NULL;
@@ -274,8 +274,12 @@ add_files_recursive (notmuch_database_t *notmuch,
 
     fs_mtime = st.st_mtime;
 
-    directory = notmuch_database_get_directory (notmuch, path);
-    db_mtime = notmuch_directory_get_mtime (directory);
+    status = notmuch_database_get_directory (notmuch, path, &directory);
+    if (status) {
+	ret = status;
+	goto DONE;
+    }
+    db_mtime = directory ? notmuch_directory_get_mtime (directory) : 0;
 
     new_directory = db_mtime ? FALSE : TRUE;
 
@@ -295,7 +299,7 @@ add_files_recursive (notmuch_database_t *notmuch,
      * by a new out-argument, or by recording this information and
      * providing an accessor.
      */
-    if (new_directory)
+    if (new_directory && directory)
 	notmuch_directory_set_mtime (directory, -1);
 
     /* If the database knows about this directory, then we sort based
@@ -810,7 +814,9 @@ _remove_directory (void *ctx,
     notmuch_filenames_t *files, *subdirs;
     char *absolute;
 
-    directory = notmuch_database_get_directory (notmuch, path);
+    status = notmuch_database_get_directory (notmuch, path, &directory);
+    if (status || !directory)
+	return status;
 
     for (files = notmuch_directory_get_child_files (directory);
 	 notmuch_filenames_valid (files);
@@ -981,9 +987,10 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     }
 
     for (f = add_files_state.directory_mtimes->head; f && !interrupted; f = f->next) {
+	notmuch_status_t status;
 	notmuch_directory_t *directory;
-	directory = notmuch_database_get_directory (notmuch, f->filename);
-	if (directory) {
+	status = notmuch_database_get_directory (notmuch, f->filename, &directory);
+	if (status == NOTMUCH_STATUS_SUCCESS && directory) {
 	    notmuch_directory_set_mtime (directory, f->mtime);
 	    notmuch_directory_destroy (directory);
 	}
-- 
cgit v1.2.3


From 3f3c446c40e6e7661620645f1c152968b5590f10 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Fri, 18 May 2012 00:13:38 -0400
Subject: new: Remove workaround for detecting newly created directory objects

Previously, notmuch_database_get_directory did not indicate whether or
not the returned directory object was newly created, which required a
workaround to distinguish newly created directory objects with no
child messages from directory objects that had no mtime set but did
have child messages.  Now that notmuch_database_get_directory
distinguishes whether or not the directory object exists in the
database, this workaround is no longer necessary.
---
 notmuch-new.c | 36 +++++++-----------------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index a3a8bec..72dd558 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -256,7 +256,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     notmuch_filenames_t *db_subdirs = NULL;
     time_t stat_time;
     struct stat st;
-    notmuch_bool_t is_maildir, new_directory;
+    notmuch_bool_t is_maildir;
     const char **tag;
 
     if (stat (path, &st)) {
@@ -281,33 +281,12 @@ add_files_recursive (notmuch_database_t *notmuch,
     }
     db_mtime = directory ? notmuch_directory_get_mtime (directory) : 0;
 
-    new_directory = db_mtime ? FALSE : TRUE;
-
-    /* XXX This is a temporary workaround.  If we don't update the
-     * database mtime until after processing messages in this
-     * directory, then a 0 mtime is *not* sufficient to indicate that
-     * this directory has no messages or subdirs in the database (for
-     * example, if an earlier run skipped the mtime update because
-     * fs_mtime == stat_time, or was interrupted before updating the
-     * mtime at the end).  To address this, we record a (bogus)
-     * non-zero value before processing any child messages so that a
-     * later run won't mistake this for a new directory (and, for
-     * example, fail to detect removed files and subdirs).
-     *
-     * A better solution would be for notmuch_database_get_directory
-     * to indicate if it really created a new directory or not, either
-     * by a new out-argument, or by recording this information and
-     * providing an accessor.
-     */
-    if (new_directory && directory)
-	notmuch_directory_set_mtime (directory, -1);
-
     /* If the database knows about this directory, then we sort based
      * on strcmp to match the database sorting. Otherwise, we can do
      * inode-based sorting for faster filesystem operation. */
     num_fs_entries = scandir (path, &fs_entries, 0,
-			      new_directory ?
-			      dirent_sort_inode : dirent_sort_strcmp_name);
+			      directory ?
+			      dirent_sort_strcmp_name : dirent_sort_inode);
 
     if (num_fs_entries == -1) {
 	fprintf (stderr, "Error opening directory %s: %s\n",
@@ -376,13 +355,12 @@ add_files_recursive (notmuch_database_t *notmuch,
      * being discovered until the clock catches up and the directory
      * is modified again).
      */
-    if (fs_mtime == db_mtime)
+    if (directory && fs_mtime == db_mtime)
 	goto DONE;
 
-    /* new_directory means a directory that the database has never
-     * seen before. In that case, we can simply leave db_files and
-     * db_subdirs NULL. */
-    if (!new_directory) {
+    /* If the database has never seen this directory before, we can
+     * simply leave db_files and db_subdirs NULL. */
+    if (directory) {
 	db_files = notmuch_directory_get_child_files (directory);
 	db_subdirs = notmuch_directory_get_child_directories (directory);
     }
-- 
cgit v1.2.3


From d99270c450d8f9ef3ecfbcbeeb99b581f36c9175 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Thu, 24 May 2012 18:01:11 -0400
Subject: new: Centralize file type stat-ing logic

This moves our logic to get a file's type into one function.  This has
several benefits: we can support OSes and file systems that do not
provide dirent.d_type or always return DT_UNKNOWN, complex
symlink-handling logic has been replaced by a simple stat fall-through
in one place, and the error message for un-stat-able file is more
accurate (previously, the error always mentioned directories, even
though a broken symlink is not a directory).
---
 notmuch-new.c | 103 +++++++++++++++++++++++++++++++++++-----------------------
 test/new      |   2 +-
 2 files changed, 64 insertions(+), 41 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index 72dd558..c64f1a7 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -154,6 +154,48 @@ dirent_sort_strcmp_name (const struct dirent **a, const struct dirent **b)
     return strcmp ((*a)->d_name, (*b)->d_name);
 }
 
+/* Return the type of a directory entry relative to path as a stat(2)
+ * mode.  Like stat, this follows symlinks.  Returns -1 and sets errno
+ * if the file's type cannot be determined (which includes dangling
+ * symlinks).
+ */
+static int
+dirent_type (const char *path, const struct dirent *entry)
+{
+    struct stat statbuf;
+    char *abspath;
+    int err, saved_errno;
+
+#ifdef _DIRENT_HAVE_D_TYPE
+    /* Mapping from d_type to stat mode_t.  We omit DT_LNK so that
+     * we'll fall through to stat and get the real file type. */
+    static const mode_t modes[] = {
+	[DT_BLK]  = S_IFBLK,
+	[DT_CHR]  = S_IFCHR,
+	[DT_DIR]  = S_IFDIR,
+	[DT_FIFO] = S_IFIFO,
+	[DT_REG]  = S_IFREG,
+	[DT_SOCK] = S_IFSOCK
+    };
+    if (entry->d_type < ARRAY_SIZE(modes) && modes[entry->d_type])
+	return modes[entry->d_type];
+#endif
+
+    abspath = talloc_asprintf (NULL, "%s/%s", path, entry->d_name);
+    if (!abspath) {
+	errno = ENOMEM;
+	return -1;
+    }
+    err = stat(abspath, &statbuf);
+    saved_errno = errno;
+    talloc_free (abspath);
+    if (err < 0) {
+	errno = saved_errno;
+	return -1;
+    }
+    return statbuf.st_mode & S_IFMT;
+}
+
 /* Test if the directory looks like a Maildir directory.
  *
  * Search through the array of directory entries to see if we can find all
@@ -162,12 +204,12 @@ dirent_sort_strcmp_name (const struct dirent **a, const struct dirent **b)
  * Return 1 if the directory looks like a Maildir and 0 otherwise.
  */
 static int
-_entries_resemble_maildir (struct dirent **entries, int count)
+_entries_resemble_maildir (const char *path, struct dirent **entries, int count)
 {
     int i, found = 0;
 
     for (i = 0; i < count; i++) {
-	if (entries[i]->d_type != DT_DIR && entries[i]->d_type != DT_UNKNOWN)
+	if (dirent_type (path, entries[i]) != S_IFDIR)
 	    continue;
 
 	if (strcmp(entries[i]->d_name, "new") == 0 ||
@@ -250,7 +292,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     notmuch_status_t status, ret = NOTMUCH_STATUS_SUCCESS;
     notmuch_message_t *message = NULL;
     struct dirent **fs_entries = NULL;
-    int i, num_fs_entries = 0;
+    int i, num_fs_entries = 0, entry_type;
     notmuch_directory_t *directory;
     notmuch_filenames_t *db_files = NULL;
     notmuch_filenames_t *db_subdirs = NULL;
@@ -300,7 +342,7 @@ add_files_recursive (notmuch_database_t *notmuch,
     }
 
     /* Pass 1: Recurse into all sub-directories. */
-    is_maildir = _entries_resemble_maildir (fs_entries, num_fs_entries);
+    is_maildir = _entries_resemble_maildir (path, fs_entries, num_fs_entries);
 
     for (i = 0; i < num_fs_entries; i++) {
 	if (interrupted)
@@ -308,17 +350,16 @@ add_files_recursive (notmuch_database_t *notmuch,
 
 	entry = fs_entries[i];
 
-	/* We only want to descend into directories.
-	 * But symlinks can be to directories too, of course.
-	 *
-	 * And if the filesystem doesn't tell us the file type in the
-	 * scandir results, then it might be a directory (and if not,
-	 * then we'll stat and return immediately in the next level of
-	 * recursion). */
-	if (entry->d_type != DT_DIR &&
-	    entry->d_type != DT_LNK &&
-	    entry->d_type != DT_UNKNOWN)
-	{
+	/* We only want to descend into directories (and symlinks to
+	 * directories). */
+	entry_type = dirent_type (path, entry);
+	if (entry_type == -1) {
+	    /* Be pessimistic, e.g. so we don't lose lots of mail just
+	     * because a user broke a symlink. */
+	    fprintf (stderr, "Error reading file %s/%s: %s\n",
+		     path, entry->d_name, strerror (errno));
+	    return NOTMUCH_STATUS_FILE_ERROR;
+	} else if (entry_type != S_IFDIR) {
 	    continue;
 	}
 
@@ -407,31 +448,13 @@ add_files_recursive (notmuch_database_t *notmuch,
 	    notmuch_filenames_move_to_next (db_subdirs);
 	}
 
-	/* If we're looking at a symlink, we only want to add it if it
-	 * links to a regular file, (and not to a directory, say).
-	 *
-	 * Similarly, if the file is of unknown type (due to filesystem
-	 * limitations), then we also need to look closer.
-	 *
-	 * In either case, a stat does the trick.
-	 */
-	if (entry->d_type == DT_LNK || entry->d_type == DT_UNKNOWN) {
-	    int err;
-
-	    next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
-	    err = stat (next, &st);
-	    talloc_free (next);
-	    next = NULL;
-
-	    /* Don't emit an error for a link pointing nowhere, since
-	     * the directory-traversal pass will have already done
-	     * that. */
-	    if (err)
-		continue;
-
-	    if (! S_ISREG (st.st_mode))
-		continue;
-	} else if (entry->d_type != DT_REG) {
+	/* Only add regular files (and symlinks to regular files). */
+	entry_type = dirent_type (path, entry);
+	if (entry_type == -1) {
+	    fprintf (stderr, "Error reading file %s/%s: %s\n",
+		     path, entry->d_name, strerror (errno));
+	    return NOTMUCH_STATUS_FILE_ERROR;
+	} else if (entry_type != S_IFREG) {
 	    continue;
 	}
 
diff --git a/test/new b/test/new
index a7bc146..cab7c01 100755
--- a/test/new
+++ b/test/new
@@ -140,7 +140,7 @@ test_begin_subtest "Broken symlink aborts"
 ln -s does-not-exist "${MAIL_DIR}/broken"
 output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" \
-"Error reading directory ${MAIL_DIR}/broken: No such file or directory
+"Error reading file ${MAIL_DIR}/broken: No such file or directory
 Note: A fatal error was encountered: Something went wrong trying to read or write a file
 No new mail."
 rm "${MAIL_DIR}/broken"
-- 
cgit v1.2.3


From da170ee6573ca8a04f01ebf789250f6b4b4d3cf0 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Thu, 24 May 2012 18:01:12 -0400
Subject: new: Merge error checks from add_files and add_files_recursive

Previously, add_files_recursive could have been called on a symlink to
a non-directory.  Hence, calling it on a non-directory was not an
error, so a separate function, add_files, existed to fail loudly in
situations where the path had to be a directory.

With the new stat-ing logic, add_files_recursive is always called on
directories, so the separation of this logic is no longer necessary.
Hence, this patch moves the strict error checking previously done by
add_files into add_files_recursive.
---
 notmuch-new.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index c64f1a7..2b05605 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -308,11 +308,10 @@ add_files_recursive (notmuch_database_t *notmuch,
     }
     stat_time = time (NULL);
 
-    /* This is not an error since we may have recursed based on a
-     * symlink to a regular file, not a directory, and we don't know
-     * that until this stat. */
-    if (! S_ISDIR (st.st_mode))
-	return NOTMUCH_STATUS_SUCCESS;
+    if (! S_ISDIR (st.st_mode)) {
+	fprintf (stderr, "Error: %s is not a directory.\n", path);
+	return NOTMUCH_STATUS_FILE_ERROR;
+    }
 
     fs_mtime = st.st_mtime;
 
@@ -655,23 +654,7 @@ add_files (notmuch_database_t *notmuch,
 	   const char *path,
 	   add_files_state_t *state)
 {
-    notmuch_status_t status;
-    struct stat st;
-
-    if (stat (path, &st)) {
-	fprintf (stderr, "Error reading directory %s: %s\n",
-		 path, strerror (errno));
-	return NOTMUCH_STATUS_FILE_ERROR;
-    }
-
-    if (! S_ISDIR (st.st_mode)) {
-	fprintf (stderr, "Error: %s is not a directory.\n", path);
-	return NOTMUCH_STATUS_FILE_ERROR;
-    }
-
-    status = add_files_recursive (notmuch, path, state);
-
-    return status;
+    return add_files_recursive (notmuch, path, state);
 }
 
 /* XXX: This should be merged with the add_files function since it
-- 
cgit v1.2.3


From 4ca36441a84155571ca2e572f3f6a96af55685b1 Mon Sep 17 00:00:00 2001
From: Austin Clements <amdragon@MIT.EDU>
Date: Thu, 24 May 2012 18:01:13 -0400
Subject: new: Unify add_files and add_files_recursive

Since starting at the top of a directory tree and recursing within
that tree are now identical operations, there's no need for both
add_files and add_files_recursive.  This eliminates add_files (which
did nothing more than call add_files_recursive after the previous
patch) and renames add_files_recursive to add_files.
---
 notmuch-new.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

(limited to 'notmuch-new.c')

diff --git a/notmuch-new.c b/notmuch-new.c
index 2b05605..938ae29 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -281,9 +281,9 @@ _entry_in_ignore_list (const char *entry, add_files_state_t *state)
  *     if fs_mtime isn't the current wall-clock time.
  */
 static notmuch_status_t
-add_files_recursive (notmuch_database_t *notmuch,
-		     const char *path,
-		     add_files_state_t *state)
+add_files (notmuch_database_t *notmuch,
+	   const char *path,
+	   add_files_state_t *state)
 {
     DIR *dir = NULL;
     struct dirent *entry = NULL;
@@ -377,7 +377,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 	}
 
 	next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
-	status = add_files_recursive (notmuch, next, state);
+	status = add_files (notmuch, next, state);
 	if (status) {
 	    ret = status;
 	    goto DONE;
@@ -647,16 +647,6 @@ stop_progress_printing_timer (void)
 }
 
 
-/* This is the top-level entry point for add_files. It does a couple
- * of error checks and then calls into the recursive function. */
-static notmuch_status_t
-add_files (notmuch_database_t *notmuch,
-	   const char *path,
-	   add_files_state_t *state)
-{
-    return add_files_recursive (notmuch, path, state);
-}
-
 /* XXX: This should be merged with the add_files function since it
  * shares a lot of logic with it. */
 /* Recursively count all regular files in path and all sub-directories
-- 
cgit v1.2.3