Patchworkβ Do not segfault on empty mime parts

login
register
about
Submitter martin f. krafft
Date 2010-03-02 15:31:28
Message ID <1267543888-18134-1-git-send-email-madduck@madduck.net>
Download mbox | patch
Permalink /patch/408/
State Accepted
Commit 449a418c65fb1f9281f761aae23576900d0d3ef1
Headers show

Comments

martin f. krafft - 2010-03-02 15:31:28
notmuch previously unconditionally checked mime parts for various
properties, but not for NULL, which is the case if libgmime encounters
an empty mime part.

Upon encounter of an empty mime part, the following is printed to
stderr (the second line due to my patch):

  (process:17197): gmime-CRITICAL **: g_mime_message_get_mime_part: assertion `GMIME_IS_MESSAGE (message)' failed
  Warning: Not indexing empty mime part.

This is probably a bug that should get addressed in libgmime, but for
not, my patch is an acceptable workaround.

Signed-off-by: martin f. krafft <madduck@madduck.net>
---
 lib/index.cc |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
Carl Worth - 2010-04-13 15:55:01
On Tue,  2 Mar 2010 16:31:28 +0100, "martin f. krafft" <madduck@madduck.net> wrote:
> notmuch previously unconditionally checked mime parts for various
> properties, but not for NULL, which is the case if libgmime encounters
> an empty mime part.

Thanks for the fix.

> Upon encounter of an empty mime part, the following is printed to
> stderr (the second line due to my patch):
> 
>   (process:17197): gmime-CRITICAL **: g_mime_message_get_mime_part: assertion `GMIME_IS_MESSAGE (message)' failed
>   Warning: Not indexing empty mime part.

That message is all quite ugly. I'm perpetually annoyed by libraries
that claim to detect major problems, (even "CRITICAL" problems!), and
just print messages and march along.

Either there *is* a critical problem here, in which case the library
should halt after printing the message, (see for example, what notmuch
does with its INTERNAL_ERROR macro), or there's no problem at all and
the code should operate correctly without these obnoxious messages.

Anyway, I've put a note into TODO that we should do the following (in
order):

  1. Add a test case to trigger this message

  2. File a bug against GMime to eliminate its portion of the message

  3. Remove the notmuch portion of the message

-Carl

Patch

diff --git a/lib/index.cc b/lib/index.cc
index cf93025..0d6640b 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -336,6 +336,11 @@  _index_mime_part (notmuch_message_t *message,
     GMimeContentDisposition *disposition;
     char *body;
 
+    if (! part) {
+	fprintf (stderr, "Warning: Not indexing empty mime part.\n");
+	return;
+    }
+
     if (GMIME_IS_MULTIPART (part)) {
 	GMimeMultipart *multipart = GMIME_MULTIPART (part);
 	int i;