From: Tomi Ollila <tomi.ollila@iki.fi>
To: Tomi Valkeinen <tomi.valkeinen@iki.fi>, notmuch@notmuchmail.org
Subject: Re: [PATCH 2/2] lib: fix error handling
In-Reply-To: <1384837831-9206-2-git-send-email-tomi.valkeinen@iki.fi>
References: <1384837831-9206-1-git-send-email-tomi.valkeinen@iki.fi>
	<1384837831-9206-2-git-send-email-tomi.valkeinen@iki.fi>
User-Agent: Notmuch/0.17+34~g98b959f (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Mon, 13 Jan 2014 23:15:13 +0200
Message-ID: <m2txd7lh26.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Tue, Nov 19 2013, Tomi Valkeinen <tomi.valkeinen@iki.fi> wrote:

> Currently if a Xapian exception happens in notmuch_message_get_header,
> the exception is not caught leading to crash. In
> notmuch_message_get_date the exception is caught, but an internal error
> is raised, again leading to crash.
>
> This patch fixes the error handling by making both functions catch the
> Xapian exceptions, print an error and return NULL or 0.
>
> The 'notmuch->exception_reported' is also set, as is done elsewhere,
> even if I don't really get the idea of that field.

Althoug this adds the library fprintf() count by one this is important
and consistent fix to the library...

... whenever we get into consensus how to handle library logging it
is not big deal to convert this also to the new call too.

+1

Tomi


>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@iki.fi>
> ---
>  lib/message.cc | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/lib/message.cc b/lib/message.cc
> index 1b46379..c91f3a5 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -412,19 +412,27 @@ _notmuch_message_ensure_message_file (notmuch_message_t *message)
>  const char *
>  notmuch_message_get_header (notmuch_message_t *message, const char *header)
>  {
> -    std::string value;
> +    try {
> +	    std::string value;
>  
> -    /* Fetch header from the appropriate xapian value field if
> -     * available */
> -    if (strcasecmp (header, "from") == 0)
> -	value = message->doc.get_value (NOTMUCH_VALUE_FROM);
> -    else if (strcasecmp (header, "subject") == 0)
> -	value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
> -    else if (strcasecmp (header, "message-id") == 0)
> -	value = message->doc.get_value (NOTMUCH_VALUE_MESSAGE_ID);
> +	    /* Fetch header from the appropriate xapian value field if
> +	     * available */
> +	    if (strcasecmp (header, "from") == 0)
> +		value = message->doc.get_value (NOTMUCH_VALUE_FROM);
> +	    else if (strcasecmp (header, "subject") == 0)
> +		value = message->doc.get_value (NOTMUCH_VALUE_SUBJECT);
> +	    else if (strcasecmp (header, "message-id") == 0)
> +		value = message->doc.get_value (NOTMUCH_VALUE_MESSAGE_ID);
>  
> -    if (!value.empty())
> -	return talloc_strdup (message, value.c_str ());
> +	    if (!value.empty())
> +		return talloc_strdup (message, value.c_str ());
> +
> +    } catch (Xapian::Error &error) {
> +	fprintf (stderr, "A Xapian exception occurred when reading header: %s\n",
> +		 error.get_msg().c_str());
> +	message->notmuch->exception_reported = TRUE;
> +	return NULL;
> +    }
>  
>      /* Otherwise fall back to parsing the file */
>      _notmuch_message_ensure_message_file (message);
> @@ -766,7 +774,9 @@ notmuch_message_get_date (notmuch_message_t *message)
>      try {
>  	value = message->doc.get_value (NOTMUCH_VALUE_TIMESTAMP);
>      } catch (Xapian::Error &error) {
> -	INTERNAL_ERROR ("Failed to read timestamp value from document.");
> +	fprintf (stderr, "A Xapian exception occurred when reading date: %s\n",
> +		 error.get_msg().c_str());
> +	message->notmuch->exception_reported = TRUE;
>  	return 0;
>      }
>  
> -- 
> 1.8.3.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch