[PATCH 1/4] lib: add versions of notmuch_query_count_{message, threads} with status return
Jani Nikula
jani at nikula.org
Sat Mar 7 05:57:08 PST 2015
On Sat, 07 Mar 2015, David Bremner <david at tethera.net> wrote:
> Although I think it's a pretty bad idea to continue using the old API,
> this allows both a more gentle transition for clients of the library,
> and allows us to break one monolithic change into a series
We should probably bump LIBNOTMUCH_MINOR_VERSION for this.
See nitpicks inline below, up to you if you decide if they're worthwhile
changes, generally LGTM.
> ---
> lib/database.cc | 8 +++++++-
> lib/notmuch.h | 34 ++++++++++++++++++++++++++++++----
> lib/query.cc | 36 +++++++++++++++++++++++++++++-------
> 3 files changed, 66 insertions(+), 12 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 3974e2e..d92eec0 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -1275,7 +1275,13 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
> if (new_features &
> (NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_BOOL_FOLDER)) {
> notmuch_query_t *query = notmuch_query_create (notmuch, "");
> - total += notmuch_query_count_messages (query);
> + unsigned msg_count;
Personal preference, I always want to specify "unsigned int" instead of
just "unsigned". Same all around patches 1 and 2.
> +
> + status = notmuch_query_count_messages_st (query, &msg_count);
> + if (status)
> + goto DONE;
> +
> + total += msg_count;
> notmuch_query_destroy (query);
> }
> if (new_features & NOTMUCH_FEATURE_DIRECTORY_DOCS) {
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index 3e326bf..a0fc276 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -914,12 +914,23 @@ notmuch_threads_destroy (notmuch_threads_t *threads);
> * This function performs a search and returns Xapian's best
> * guess as to number of matching messages.
> *
> - * If a Xapian exception occurs, this function may return 0 (after
> - * printing a message).
> + * The version returning the count directly instead of a status value
> + * is deprecated.
> + *
> + * Return value (of the _st) version:
> + *
> + * NOTMUCH_STATUS_SUCCESS: query complete successfully.
> + *
> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
> + * value of *count is not defined.
> */
I'd really like to have separate doc comments for both the
functions. The original and now deprecated version can be be a brief
one, with a reference to the _st version documentation. If you move the
_st version declaration here, above the original declaration, the diff
should remain small.
Perhaps reference the library version number when this was added?
> +
> unsigned
> notmuch_query_count_messages (notmuch_query_t *query);
>
> +notmuch_status_t
> +notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count);
> +
> /**
> * Return the number of threads matching a search.
> *
> @@ -928,13 +939,28 @@ notmuch_query_count_messages (notmuch_query_t *query);
> * search.
> *
> * Note that this is a significantly heavier operation than
> - * notmuch_query_count_messages().
> + * notmuch_query_count_messages{_st}().
> + *
> + * The version returning the count directly instead of a status value
> + * is deprecated.
> *
> - * If an error occurs, this function may return 0.
> + * Return values (of the _st version):
> + *
> + * NOTMUCH_STATUS_OUT_OF_MEMORY: Memory allocation failed. The value
> + * of *count is not defined
> +
> + * NOTMUCH_STATUS_SUCCESS: query complete successfully.
> + *
> + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The
> + * value of *count is not defined.
> */
Same as above.
> +
> unsigned
> notmuch_query_count_threads (notmuch_query_t *query);
>
> +notmuch_status_t
> +notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count);
> +
> /**
> * Get the thread ID of 'thread'.
> *
> diff --git a/lib/query.cc b/lib/query.cc
> index 9279915..883d128 100644
> --- a/lib/query.cc
> +++ b/lib/query.cc
> @@ -541,6 +541,16 @@ notmuch_threads_destroy (notmuch_threads_t *threads)
> unsigned
> notmuch_query_count_messages (notmuch_query_t *query)
> {
> + notmuch_status_t status;
> + unsigned count;
> +
> + status = notmuch_query_count_messages_st (query, &count);
> + return status ? 0 : count;
> +}
> +
> +notmuch_status_t
> +notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count_out)
> +{
> notmuch_database_t *notmuch = query->notmuch;
> const char *query_string = query->query_string;
> Xapian::doccount count = 0;
> @@ -595,30 +605,42 @@ notmuch_query_count_messages (notmuch_query_t *query)
> fprintf (stderr, "A Xapian exception occurred: %s\n",
> error.get_msg().c_str());
> fprintf (stderr, "Query string was: %s\n", query->query_string);
> + return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
> }
>
> - return count;
> + *count_out=count;
Spaces around "="?
> + return NOTMUCH_STATUS_SUCCESS;
> }
>
> unsigned
> notmuch_query_count_threads (notmuch_query_t *query)
> {
> + notmuch_status_t status;
> + unsigned count;
> +
> + status = notmuch_query_count_threads_st (query, &count);
> + return status ? 0 : count;
> +}
> +
> +notmuch_status_t
> +notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count)
> +{
> notmuch_messages_t *messages;
> GHashTable *hash;
> - unsigned int count;
> notmuch_sort_t sort;
> + notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
>
> sort = query->sort;
> query->sort = NOTMUCH_SORT_UNSORTED;
> messages = notmuch_query_search_messages (query);
> query->sort = sort;
> if (messages == NULL)
> - return 0;
> + return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
>
> hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);
> if (hash == NULL) {
> talloc_free (messages);
> - return 0;
> + return NOTMUCH_STATUS_OUT_OF_MEMORY;
> }
>
> while (notmuch_messages_valid (messages)) {
> @@ -627,7 +649,7 @@ notmuch_query_count_threads (notmuch_query_t *query)
> char *thread_id_copy = talloc_strdup (messages, thread_id);
> if (unlikely (thread_id_copy == NULL)) {
> notmuch_message_destroy (message);
> - count = 0;
> + ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
> goto DONE;
> }
> g_hash_table_insert (hash, thread_id_copy, NULL);
> @@ -635,11 +657,11 @@ notmuch_query_count_threads (notmuch_query_t *query)
> notmuch_messages_move_to_next (messages);
> }
>
> - count = g_hash_table_size (hash);
> + *count = g_hash_table_size (hash);
>
> DONE:
> g_hash_table_unref (hash);
> talloc_free (messages);
>
> - return count;
> + return ret;
> }
> --
> 2.1.4
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
More information about the notmuch
mailing list