[PATCH] cli: propagate NULL from _mime_node_create, handle it in callers.

Subject: [PATCH] cli: propagate NULL from _mime_node_create, handle it in callers.

Date: Thu, 31 Aug 2017 21:10:14 -0300

To: Jan Malakhovski, notmuch@notmuchmail.org

Cc:

From: David Bremner


Under certain error conditions _mime_node_create can return NULL. This
was ignored in mime_node_child, which was clearly an error. This
commit propagates that NULL from mime_node_child, and attempts to deal
with the consequences.  I chose to push the checks for a null mime
node into the format_blah functions on the grounds that at least in in
the sprinter version we could in principle include error reporting in
the output. Leave that for now as I lack a test case, and I think it
would require changing the output schema.

Based heavily on a patch from Jan Malakhovski.
---
Hi Jan;

Can you check if this fixes the segfault for you? It's based on your
patch, and your followup comment. It tries to handle all of the places
mime_node_child is called.

 mime-node.c     | 13 +++++++++----
 notmuch-reply.c |  4 ++++
 notmuch-show.c  |  8 ++++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/mime-node.c b/mime-node.c
index 24d73afa..349a699b 100644
--- a/mime-node.c
+++ b/mime-node.c
@@ -317,7 +317,10 @@ mime_node_child (mime_node_t *parent, int child)
 	INTERNAL_ERROR ("Unexpected GMimeObject type: %s",
 			g_type_name (G_OBJECT_TYPE (parent->part)));
     }
+
     node = _mime_node_create (parent, sub);
+    if (!node)
+	return NULL;
 
     if (child == parent->next_child && parent->next_part_num != -1) {
 	/* We're traversing in depth-first order.  Record the child's
@@ -354,11 +357,13 @@ _mime_node_seek_dfs_walk (mime_node_t *node, int *n)
     *n -= 1;
     for (i = 0; i < node->nchildren; i++) {
 	mime_node_t *child = mime_node_child (node, i);
-	mime_node_t *ret = _mime_node_seek_dfs_walk (child, n);
-	if (ret)
-	    return ret;
+	if (child) {
+	    mime_node_t *ret = _mime_node_seek_dfs_walk (child, n);
+	    if (ret)
+		return ret;
 
-	talloc_free (child);
+	    talloc_free (child);
+	}
     }
     return NULL;
 }
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 929f3077..8498b09e 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -38,6 +38,10 @@ format_part_reply (GMimeStream *stream, mime_node_t *node)
 {
     int i;
 
+    /* XXX Print some diagnostic? XXX */
+    if (!node)
+	return;
+
     if (node->envelope_file) {
 	g_mime_stream_printf (stream, "On %s, %s wrote:\n",
 			      notmuch_message_get_header (node->envelope_file, "date"),
diff --git a/notmuch-show.c b/notmuch-show.c
index cdcc2a98..28c957c1 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -495,6 +495,10 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
     const char *part_type;
     int i;
 
+    /* XXX print some diagnostic? XXX*/
+    if (!node)
+	return;
+
     if (node->envelope_file) {
 	notmuch_message_t *message = node->envelope_file;
 
@@ -609,6 +613,10 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node,
     /* Any changes to the JSON or S-Expression format should be
      * reflected in the file devel/schemata. */
 
+    /* XXX represent NULL node in output? XXX */
+    if (!node)
+	return;
+
     if (node->envelope_file) {
 	sp->begin_map (sp);
 	format_message_sprinter (sp, node->envelope_file);
-- 
2.14.1

_______________________________________________
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

Thread: