unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* v2 pre-gmime-3.0 cleanup
@ 2017-05-23  0:53 David Bremner
  2017-05-23  0:53 ` [Patch v2 1/7] test/multipart: reorganize creation of multipart message David Bremner
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

this obsoletes the series at

     id:20170521124821.23924-1-david@tethera.net

Compared to the previous series, it contains a leak fix for
g_mime_stream_stdout_new(), the actual leak fixes promised in the last
patch of the previous series, and modifications to some of the
multipart tests so that headers are no longer expected when printing
multipart bodies; as discussed in

	  id:878tlpyyfi.fsf@tethera.net

and confirmed with gmime upstream, that behaviour can be considered a
bug in gmime-2.6, and goes away in gmime-3.0

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Patch v2 1/7] test/multipart: reorganize creation of multipart message
  2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
@ 2017-05-23  0:53 ` David Bremner
  2017-05-23  0:53 ` [Patch v2 2/7] test: mark inclusion of headers with raw body as bug David Bremner
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

We want to have the bodies of the multipart available in a file on
their own for planned modifications to tests.
---
 test/T190-multipart.sh | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
index 12a10451..91a632c6 100755
--- a/test/T190-multipart.sh
+++ b/test/T190-multipart.sh
@@ -2,14 +2,7 @@
 test_description="output of multipart message"
 . ./test-lib.sh || exit 1
 
-cat <<EOF > embedded_message
-From: Carl Worth <cworth@cworth.org>
-To: cworth@cworth.org
-Subject: html message
-Date: Fri, 05 Jan 2001 15:42:57 +0000
-User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
-Message-ID: <87liy5ap01.fsf@yoom.home.cworth.org>
-MIME-Version: 1.0
+cat <<EOF > embedded_message_body
 Content-Type: multipart/alternative; boundary="==-=-=="
 
 --==-=-==
@@ -24,15 +17,19 @@ This is an embedded message, with a multipart/alternative part.
 
 --==-=-==--
 EOF
-
-cat <<EOF > ${MAIL_DIR}/multipart
+cat <<EOF > embedded_message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: Multipart message
-Date: Fri, 05 Jan 2001 15:43:57 +0000
+Subject: html message
+Date: Fri, 05 Jan 2001 15:42:57 +0000
 User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
-Message-ID: <87liy5ap00.fsf@yoom.home.cworth.org>
+Message-ID: <87liy5ap01.fsf@yoom.home.cworth.org>
 MIME-Version: 1.0
+EOF
+
+cat embedded_message_body >> embedded_message
+
+cat <<EOF > multipart_body
 Content-Type: multipart/signed; boundary="==-=-=";
 	micalg=pgp-sha1; protocol="application/pgp-signature"
 
@@ -44,8 +41,9 @@ Content-Type: message/rfc822
 Content-Disposition: inline
 
 EOF
-cat embedded_message >> ${MAIL_DIR}/multipart
-cat <<EOF >> ${MAIL_DIR}/multipart
+
+cat embedded_message >> multipart_body
+cat <<EOF >> multipart_body
 
 --=-=-=
 Content-Disposition: attachment; filename=attachment
@@ -73,6 +71,18 @@ W6cAmQE4dcYrx/LPLtYLZm1jsGauE5hE
 --==-=-=--
 EOF
 
+cat <<EOF > ${MAIL_DIR}/multipart
+From: Carl Worth <cworth@cworth.org>
+To: cworth@cworth.org
+Subject: Multipart message
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
+Message-ID: <87liy5ap00.fsf@yoom.home.cworth.org>
+MIME-Version: 1.0
+EOF
+
+cat multipart_body >> ${MAIL_DIR}/multipart
+
 cat <<EOF > ${MAIL_DIR}/base64-part-with-crlf
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch v2 2/7] test: mark inclusion of headers with raw body as bug
  2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
  2017-05-23  0:53 ` [Patch v2 1/7] test/multipart: reorganize creation of multipart message David Bremner
@ 2017-05-23  0:53 ` David Bremner
  2017-05-23  0:53 ` [Patch v2 3/7] util: convenience function to create gmime stream for stdout David Bremner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

The output of headers here reflects an underlying bug / quirk of
gmime-2.6.
---
 test/T190-multipart.sh | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
index 91a632c6..e2e488a9 100755
--- a/test/T190-multipart.sh
+++ b/test/T190-multipart.sh
@@ -465,8 +465,9 @@ notmuch show --format=raw --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUT
 test_expect_equal_file "${MAIL_DIR}"/multipart OUTPUT
 
 test_begin_subtest "--format=raw --part=1, message body"
+test_subtest_known_broken
 notmuch show --format=raw --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-test_expect_equal_file "${MAIL_DIR}"/multipart OUTPUT
+test_expect_equal_file multipart_body OUTPUT
 
 test_begin_subtest "--format=raw --part=2, multipart/mixed"
 notmuch show --format=raw --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
@@ -518,30 +519,9 @@ notmuch show --format=raw --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUT
 test_expect_equal_file embedded_message OUTPUT
 
 test_begin_subtest "--format=raw --part=4, rfc822's multipart"
+test_subtest_known_broken
 notmuch show --format=raw --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-cat <<EOF >EXPECTED
-From: Carl Worth <cworth@cworth.org>
-To: cworth@cworth.org
-Subject: html message
-Date: Fri, 05 Jan 2001 15:42:57 +0000
-User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
-Message-ID: <87liy5ap01.fsf@yoom.home.cworth.org>
-MIME-Version: 1.0
-Content-Type: multipart/alternative; boundary="==-=-=="
-
---==-=-==
-Content-Type: text/html
-
-<p>This is an embedded message, with a multipart/alternative part.</p>
-
---==-=-==
-Content-Type: text/plain
-
-This is an embedded message, with a multipart/alternative part.
-
---==-=-==--
-EOF
-test_expect_equal_file EXPECTED OUTPUT
+test_expect_equal_file embedded_message_body OUTPUT
 
 test_begin_subtest "--format=raw --part=5, rfc822's html part"
 notmuch show --format=raw --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch v2 3/7] util: convenience function to create gmime stream for stdout
  2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
  2017-05-23  0:53 ` [Patch v2 1/7] test/multipart: reorganize creation of multipart message David Bremner
  2017-05-23  0:53 ` [Patch v2 2/7] test: mark inclusion of headers with raw body as bug David Bremner
@ 2017-05-23  0:53 ` David Bremner
  2017-05-23  0:53 ` [Patch v2 4/7] cli/reply: direct all output for text format to gmime stream David Bremner
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

It turns out that our use of GMimeStreamPipe has only succeeded
because gmime has been ignoring some seek failures; this will no
longer be the case in gmime 3.0, so we use a GMimeStreamPipe, which
does not assume seekability, wrapped in a buffering stream.
---
 util/Makefile.local |  2 +-
 util/gmime-extra.c  | 20 ++++++++++++++++++++
 util/gmime-extra.h  |  6 ++++++
 3 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 util/gmime-extra.c
 create mode 100644 util/gmime-extra.h

diff --git a/util/Makefile.local b/util/Makefile.local
index a6962d49..3027880b 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -5,7 +5,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c \
-		$(dir)/util.c
+		$(dir)/util.c $(dir)/gmime-extra.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
 
diff --git a/util/gmime-extra.c b/util/gmime-extra.c
new file mode 100644
index 00000000..f1538587
--- /dev/null
+++ b/util/gmime-extra.c
@@ -0,0 +1,20 @@
+#include "gmime-extra.h"
+
+GMimeStream *
+g_mime_stream_stdout_new()
+{
+    GMimeStream *stream_stdout = NULL;
+    GMimeStream *stream_buffered = NULL;
+
+    stream_stdout = g_mime_stream_pipe_new (STDOUT_FILENO);
+    if (!stream_stdout)
+	return NULL;
+
+    g_mime_stream_pipe_set_owner (GMIME_STREAM_PIPE (stream_stdout), FALSE);
+
+    stream_buffered = g_mime_stream_buffer_new (stream_stdout, GMIME_STREAM_BUFFER_BLOCK_WRITE);
+
+    g_object_unref (stream_stdout);
+
+    return stream_buffered;
+}
diff --git a/util/gmime-extra.h b/util/gmime-extra.h
new file mode 100644
index 00000000..e0432a94
--- /dev/null
+++ b/util/gmime-extra.h
@@ -0,0 +1,6 @@
+#ifndef _GMIME_EXTRA_H
+#define _GMIME_EXTRA_H
+#include <gmime/gmime.h>
+
+GMimeStream *g_mime_stream_stdout_new(void);
+#endif
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch v2 4/7] cli/reply: direct all output for text format to gmime stream
  2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
                   ` (2 preceding siblings ...)
  2017-05-23  0:53 ` [Patch v2 3/7] util: convenience function to create gmime stream for stdout David Bremner
@ 2017-05-23  0:53 ` David Bremner
  2017-05-23  0:53 ` [Patch v2 5/7] cli/show: use single stream for printf / gmime object output David Bremner
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

Interleaving printfs with writes to the gmime stream worked when the
gmime stream was backed by the FILE *stdout, but that is no longer the
case.  Create one stream and pass it into the two functions where
needed, as well well as replacing printfs with g_mime_stream_printf.
---
 notmuch-client.h |  2 +-
 notmuch-reply.c  | 63 +++++++++++++++++++++++++++-----------------------------
 2 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index a6f70eae..5692caf3 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -29,7 +29,7 @@
 
 #include "compat.h"
 
-#include <gmime/gmime.h>
+#include "gmime-extra.h"
 
 typedef GMimeCryptoContext notmuch_crypto_context_t;
 /* This is automatically included only since gmime 2.6.10 */
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2fa6e5a3..9239aac2 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -25,47 +25,42 @@
 #include "sprinter.h"
 
 static void
-show_reply_headers (GMimeMessage *message)
+show_reply_headers (GMimeStream *stream, GMimeMessage *message)
 {
-    GMimeStream *stream_stdout = NULL;
-
-    stream_stdout = g_mime_stream_file_new (stdout);
-    if (stream_stdout) {
-	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	/* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
-	g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_stdout);
-	g_object_unref(stream_stdout);
+    /* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
+    if (g_mime_object_write_to_stream (GMIME_OBJECT(message), stream) < 0) {
+	INTERNAL_ERROR("failed to write headers to stdout\n");
     }
 }
 
 static void
-format_part_reply (mime_node_t *node)
+format_part_reply (GMimeStream *stream, mime_node_t *node)
 {
     int i;
 
     if (node->envelope_file) {
-	printf ("On %s, %s wrote:\n",
-		notmuch_message_get_header (node->envelope_file, "date"),
-		notmuch_message_get_header (node->envelope_file, "from"));
+	g_mime_stream_printf (stream, "On %s, %s wrote:\n",
+			      notmuch_message_get_header (node->envelope_file, "date"),
+			      notmuch_message_get_header (node->envelope_file, "from"));
     } else if (GMIME_IS_MESSAGE (node->part)) {
 	GMimeMessage *message = GMIME_MESSAGE (node->part);
 	InternetAddressList *recipients;
 	const char *recipients_string;
 
-	printf ("> From: %s\n", g_mime_message_get_sender (message));
+	g_mime_stream_printf (stream, "> From: %s\n", g_mime_message_get_sender (message));
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("> To: %s\n",
-		    recipients_string);
+	    g_mime_stream_printf (stream, "> To: %s\n",
+				  recipients_string);
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("> Cc: %s\n",
-		    recipients_string);
-	printf ("> Subject: %s\n", g_mime_message_get_subject (message));
-	printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
-	printf (">\n");
+	    g_mime_stream_printf (stream, "> Cc: %s\n",
+				  recipients_string);
+	g_mime_stream_printf (stream, "> Subject: %s\n", g_mime_message_get_subject (message));
+	g_mime_stream_printf (stream, "> Date: %s\n", g_mime_message_get_date_as_string (message));
+	g_mime_stream_printf (stream, ">\n");
     } else if (GMIME_IS_PART (node->part)) {
 	GMimeContentType *content_type = g_mime_object_get_content_type (node->part);
 	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (node->part);
@@ -75,24 +70,21 @@ format_part_reply (mime_node_t *node)
 	    /* Ignore PGP/MIME cruft parts */
 	} else if (g_mime_content_type_is_type (content_type, "text", "*") &&
 		   !g_mime_content_type_is_type (content_type, "text", "html")) {
-	    GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
-	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    show_text_part_content (node->part, stream_stdout, NOTMUCH_SHOW_TEXT_PART_REPLY);
-	    g_object_unref(stream_stdout);
+	    show_text_part_content (node->part, stream, NOTMUCH_SHOW_TEXT_PART_REPLY);
 	} else if (disposition &&
 		   strcasecmp (g_mime_content_disposition_get_disposition (disposition),
 			       GMIME_DISPOSITION_ATTACHMENT) == 0) {
 	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
-	    printf ("Attachment: %s (%s)\n", filename,
-		    g_mime_content_type_to_string (content_type));
+	    g_mime_stream_printf (stream, "Attachment: %s (%s)\n", filename,
+				  g_mime_content_type_to_string (content_type));
 	} else {
-	    printf ("Non-text part: %s\n",
-		    g_mime_content_type_to_string (content_type));
+	    g_mime_stream_printf (stream, "Non-text part: %s\n",
+				  g_mime_content_type_to_string (content_type));
 	}
     }
 
     for (i = 0; i < node->nchildren; i++)
-	format_part_reply (mime_node_child (node, i));
+	format_part_reply (stream, mime_node_child (node, i));
 }
 
 typedef enum {
@@ -678,9 +670,14 @@ static int do_reply(notmuch_config_t *config,
 	    /* End */
 	    sp->end (sp);
 	} else {
-	    show_reply_headers (reply);
-	    if (format == FORMAT_DEFAULT)
-		format_part_reply (node);
+	    GMimeStream *stream_stdout = stream_stdout = g_mime_stream_stdout_new ();
+	    if (stream_stdout) {
+		show_reply_headers (stream_stdout, reply);
+		if (format == FORMAT_DEFAULT)
+		    format_part_reply (stream_stdout, node);
+	    }
+	    g_mime_stream_flush (stream_stdout);
+	    g_object_unref(stream_stdout);
 	}
 
 	g_object_unref (G_OBJECT (reply));
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch v2 5/7] cli/show: use single stream for printf / gmime object output
  2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
                   ` (3 preceding siblings ...)
  2017-05-23  0:53 ` [Patch v2 4/7] cli/reply: direct all output for text format to gmime stream David Bremner
@ 2017-05-23  0:53 ` David Bremner
  2017-05-23  0:53 ` [Patch v2 6/7] perf-test: add memory test for reply David Bremner
  2017-05-23  0:53 ` [Patch v2 7/7] cli/reply: fix two memory leaks, document a third David Bremner
  6 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

This is again motivated by the need to transition away from
GMimeStreamFile for output to stdout.

format_part_mbox is left alone for now, as this cannot be mixed in
with output using gmime object output.
---
 notmuch-client.h |  1 +
 notmuch-show.c   | 70 ++++++++++++++++++++++++++------------------------------
 2 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 5692caf3..62d4bcec 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -86,6 +86,7 @@ typedef struct notmuch_show_params {
     int part;
     notmuch_crypto_t crypto;
     notmuch_bool_t include_html;
+    GMimeStream *out_stream;
 } notmuch_show_params_t;
 
 /* There's no point in continuing when we've detected that we've done
diff --git a/notmuch-show.c b/notmuch-show.c
index 7021008e..accea48a 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -426,6 +426,7 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	GMIME_OBJECT (node->envelope_part) : node->part;
     GMimeContentType *content_type = g_mime_object_get_content_type (meta);
     const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
+    GMimeStream *stream = params->out_stream;
     const char *part_type;
     int i;
 
@@ -433,13 +434,13 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	notmuch_message_t *message = node->envelope_file;
 
 	part_type = "message";
-	printf ("\f%s{ id:%s depth:%d match:%d excluded:%d filename:%s\n",
-		part_type,
-		notmuch_message_get_message_id (message),
-		indent,
-		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
-		notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
-		notmuch_message_get_filename (message));
+	g_mime_stream_printf (stream, "\f%s{ id:%s depth:%d match:%d excluded:%d filename:%s\n",
+			      part_type,
+			      notmuch_message_get_message_id (message),
+			      indent,
+			      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
+			      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
+			      notmuch_message_get_filename (message));
     } else {
 	char *content_string;
 	const char *disposition = _get_disposition (meta);
@@ -453,14 +454,14 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	else
 	    part_type = "part";
 
-	printf ("\f%s{ ID: %d", part_type, node->part_num);
+	g_mime_stream_printf (stream, "\f%s{ ID: %d", part_type, node->part_num);
 	if (filename)
-	    printf (", Filename: %s", filename);
+	    g_mime_stream_printf (stream, ", Filename: %s", filename);
 	if (cid)
-	    printf (", Content-id: %s", cid);
+	    g_mime_stream_printf (stream, ", Content-id: %s", cid);
 
 	content_string = g_mime_content_type_to_string (content_type);
-	printf (", Content-type: %s\n", content_string);
+	g_mime_stream_printf (stream, ", Content-type: %s\n", content_string);
 	g_free (content_string);
     }
 
@@ -470,40 +471,37 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	char *recipients_string;
 	char *date_string;
 
-	printf ("\fheader{\n");
+	g_mime_stream_printf (stream, "\fheader{\n");
 	if (node->envelope_file)
-	    printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
-	printf ("Subject: %s\n", g_mime_message_get_subject (message));
-	printf ("From: %s\n", g_mime_message_get_sender (message));
+	    g_mime_stream_printf (stream, "%s\n", _get_one_line_summary (ctx, node->envelope_file));
+	g_mime_stream_printf (stream, "Subject: %s\n", g_mime_message_get_subject (message));
+	g_mime_stream_printf (stream, "From: %s\n", g_mime_message_get_sender (message));
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("To: %s\n", recipients_string);
+	    g_mime_stream_printf (stream, "To: %s\n", recipients_string);
 	g_free (recipients_string);
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("Cc: %s\n", recipients_string);
+	    g_mime_stream_printf (stream, "Cc: %s\n", recipients_string);
 	g_free (recipients_string);
 	date_string = g_mime_message_get_date_as_string (message);
-	printf ("Date: %s\n", date_string);
+	g_mime_stream_printf (stream, "Date: %s\n", date_string);
 	g_free (date_string);
-	printf ("\fheader}\n");
+	g_mime_stream_printf (stream, "\fheader}\n");
 
-	printf ("\fbody{\n");
+	g_mime_stream_printf (stream, "\fbody{\n");
     }
 
     if (leaf) {
 	if (g_mime_content_type_is_type (content_type, "text", "*") &&
 	    !g_mime_content_type_is_type (content_type, "text", "html"))
 	{
-	    GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
-	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    show_text_part_content (node->part, stream_stdout, 0);
-	    g_object_unref(stream_stdout);
+	    show_text_part_content (node->part, stream, 0);
 	} else {
 	    char *content_string = g_mime_content_type_to_string (content_type);
-	    printf ("Non-text part: %s\n", content_string);
+	    g_mime_stream_printf (stream, "Non-text part: %s\n", content_string);
 	    g_free (content_string);
 	}
     }
@@ -512,9 +510,9 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	format_part_text (ctx, sp, mime_node_child (node, i), indent, params);
 
     if (GMIME_IS_MESSAGE (node->part))
-	printf ("\fbody}\n");
+	g_mime_stream_printf (stream, "\fbody}\n");
 
-    printf ("\f%s}\n", part_type);
+    g_mime_stream_printf (stream, "\f%s}\n", part_type);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
@@ -741,7 +739,7 @@ format_part_mbox (const void *ctx, unused (sprinter_t *sp), mime_node_t *node,
 static notmuch_status_t
 format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
 		 mime_node_t *node, unused (int indent),
-		 unused (const notmuch_show_params_t *params))
+		 const notmuch_show_params_t *params)
 {
     if (node->envelope_file) {
 	/* Special case the entire message to avoid MIME parsing. */
@@ -781,13 +779,7 @@ format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
 	return NOTMUCH_STATUS_SUCCESS;
     }
 
-    GMimeStream *stream_stdout;
-    GMimeStream *stream_filter = NULL;
-
-    stream_stdout = g_mime_stream_file_new (stdout);
-    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-
-    stream_filter = g_mime_stream_filter_new (stream_stdout);
+    GMimeStream *stream_filter = g_mime_stream_filter_new (params->out_stream);
 
     if (GMIME_IS_PART (node->part)) {
 	/* For leaf parts, we emit only the transfer-decoded
@@ -810,9 +802,6 @@ format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
     if (stream_filter)
 	g_object_unref (stream_filter);
 
-    if (stream_stdout)
-	g_object_unref(stream_stdout);
-
     return NOTMUCH_STATUS_SUCCESS;
 }
 
@@ -1167,6 +1156,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     formatter = formatters[format];
     sprinter = formatter->new_sprinter(config, stdout);
 
+    params.out_stream = g_mime_stream_stdout_new ();
+
     /* If a single message is requested we do not use search_excludes. */
     if (single_message) {
 	ret = do_show_single (config, query, formatter, sprinter, &params);
@@ -1200,6 +1191,9 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
  DONE:
+    g_mime_stream_flush (params.out_stream);
+    g_object_unref (params.out_stream);
+
     notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch v2 6/7] perf-test: add memory test for reply
  2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
                   ` (4 preceding siblings ...)
  2017-05-23  0:53 ` [Patch v2 5/7] cli/show: use single stream for printf / gmime object output David Bremner
@ 2017-05-23  0:53 ` David Bremner
  2017-05-23  0:53 ` [Patch v2 7/7] cli/reply: fix two memory leaks, document a third David Bremner
  6 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

Looking at the code for notmuch-reply, there seems to be several gmime
related memory leaks. This test is supposed to help eliminate those.
---
 performance-test/M04-reply.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100755 performance-test/M04-reply.sh

diff --git a/performance-test/M04-reply.sh b/performance-test/M04-reply.sh
new file mode 100755
index 00000000..1aaf00f5
--- /dev/null
+++ b/performance-test/M04-reply.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+test_description='search'
+
+. ./perf-test-lib.sh || exit 1
+
+memory_start
+
+for id in $(notmuch search --output=messages '*' | shuf -n 5); do
+    memory_run "reply $id" "notmuch reply $id 1>/dev/null"
+    memory_run "reply --format=json $id" "notmuch reply --format=json \"$id\" 1>/dev/null"
+    memory_run "reply --format=sexp $id" "notmuch reply --format=sexp \"$id\" 1>/dev/null"
+done
+
+memory_done
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Patch v2 7/7] cli/reply: fix two memory leaks, document a third
  2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
                   ` (5 preceding siblings ...)
  2017-05-23  0:53 ` [Patch v2 6/7] perf-test: add memory test for reply David Bremner
@ 2017-05-23  0:53 ` David Bremner
  2017-05-24  0:50   ` [PATCH] test/thread-naming: remove excess escaping from sender address David Bremner
  6 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2017-05-23  0:53 UTC (permalink / raw)
  To: notmuch, notmuch

internet_address_list_to_string returns an allocated string, which
needs to be freed with g_free.

The third leak we leave as it would require restructuring of
add_recipients_from_message, and is fixed by later gmime-3.0 porting.
---
 notmuch-reply.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9239aac2..b88f1d31 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -45,7 +45,7 @@ format_part_reply (GMimeStream *stream, mime_node_t *node)
     } else if (GMIME_IS_MESSAGE (node->part)) {
 	GMimeMessage *message = GMIME_MESSAGE (node->part);
 	InternetAddressList *recipients;
-	const char *recipients_string;
+	char *recipients_string;
 
 	g_mime_stream_printf (stream, "> From: %s\n", g_mime_message_get_sender (message));
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
@@ -53,11 +53,13 @@ format_part_reply (GMimeStream *stream, mime_node_t *node)
 	if (recipients_string)
 	    g_mime_stream_printf (stream, "> To: %s\n",
 				  recipients_string);
+	g_free (recipients_string);
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
 	    g_mime_stream_printf (stream, "> Cc: %s\n",
 				  recipients_string);
+	g_free (recipients_string);
 	g_mime_stream_printf (stream, "> Subject: %s\n", g_mime_message_get_subject (message));
 	g_mime_stream_printf (stream, "> Date: %s\n", g_mime_message_get_date_as_string (message));
 	g_mime_stream_printf (stream, ">\n");
@@ -329,6 +331,12 @@ add_recipients_from_message (GMimeMessage *reply,
 			     GMimeMessage *message,
 			     notmuch_bool_t reply_all)
 {
+
+    /* There is a memory leak here with gmime-2.6 because get_sender
+     * returns a newly allocated list, while the others return
+     * references to libgmime owned data. This leak will be fixed with
+     * the transition to gmime-3.0.
+     */
     struct {
 	InternetAddressList * (*get_header)(GMimeMessage *message);
 	GMimeRecipientType recipient_type;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] test/thread-naming: remove excess escaping from sender address.
  2017-05-23  0:53 ` [Patch v2 7/7] cli/reply: fix two memory leaks, document a third David Bremner
@ 2017-05-24  0:50   ` David Bremner
  2017-05-24 23:53     ` [PATCH] test: test parsing of malformed from addresses David Bremner
  0 siblings, 1 reply; 10+ messages in thread
From: David Bremner @ 2017-05-24  0:50 UTC (permalink / raw)
  To: David Bremner, notmuch, notmuch

This is another case where the behaviour of gmime-2.6 and gmime-3.0
seems to differ. It may be that we prefer the more lax parsing of the
previous version, but that should be tested seperately.
---

As far as I can tell, the "LOOSE" parsing in gmime-3.0 is not as loose
as previous. It could well be that in the previous version we were
passing the raw header to xapian for term generation. That's arguably
the right thing to do, but as hinted in the commit message, I'd rather
deal with that in a seperate commit.

 test/T200-thread-naming.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/T200-thread-naming.sh b/test/T200-thread-naming.sh
index 132c1d77..2167ba8e 100755
--- a/test/T200-thread-naming.sh
+++ b/test/T200-thread-naming.sh
@@ -66,11 +66,11 @@ test_expect_equal "$output" "thread:XXX   2001-01-12 [6/8] Notmuch Test Suite; t
 test_begin_subtest "Use empty subjects if necessary."
 add_message '[subject]="@FORCE_EMPTY"' \
 	    '[date]="Sat, 13 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"'
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"'
 empty_parent=${gen_msg_id}
 add_message '[subject]="@FORCE_EMPTY"' \
 	    '[date]="Sun, 14 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 output=$(notmuch search --sort=newest-first from:empty_test@notmuchmail.org | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-14 [2/2] Empty Sender;  (inbox unread)"
@@ -78,15 +78,15 @@ test_expect_equal "$output" "thread:XXX   2001-01-14 [2/2] Empty Sender;  (inbox
 test_begin_subtest "Avoid empty subjects if possible (newest-first)."
 add_message '[subject]="Non-empty subject (1)"' \
 	    '[date]="Mon, 15 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 add_message '[subject]="Non-empty subject (2)"' \
 	    '[date]="Mon, 16 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 add_message '[subject]="@FORCE_EMPTY"' \
 	    '[date]="Tue, 17 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 output=$(notmuch search --sort=newest-first from:Empty | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-17 [5/5] Empty Sender; Non-empty subject (2) (inbox unread)"
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH] test: test parsing of malformed from addresses
  2017-05-24  0:50   ` [PATCH] test/thread-naming: remove excess escaping from sender address David Bremner
@ 2017-05-24 23:53     ` David Bremner
  0 siblings, 0 replies; 10+ messages in thread
From: David Bremner @ 2017-05-24 23:53 UTC (permalink / raw)
  To: David Bremner, notmuch, notmuch

This was previously tested in T200-thread-naming.sh, but failures due
to changes in address parsing were confusing because they had nothing
to do with threads.
---

At the moment this is broken with gmime-3.0, and I'm not sure it's
really easily fixable:

   https://mail.gnome.org/archives/gmime-devel-list/2017-May/msg00008.html

 test/T050-new.sh | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index ffa303ef..18291ce5 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -140,6 +140,12 @@ ln -s "$external_msg_filename" "$gen_msg_filename"
 output=$(NOTMUCH_NEW --debug)
 test_expect_equal "$output" "Added 1 new message to the database."
 
+test_begin_subtest "Index malformed from address."
+add_message '[subject]="test subject"' \
+            '[date]="Sat, 13 Jan 2001 15:43:45 -0000"' \
+            '[from]="Malformed From \<malformed_from@notmuchmail.org\>"'
+output=$(notmuch search --sort=newest-first from:malformed_from@notmuchmail.org | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-13 [1/1] Malformed From; test subject (inbox unread)"
 
 test_begin_subtest "Broken symlink aborts"
 ln -s does-not-exist "${MAIL_DIR}/broken"
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-05-24 23:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-23  0:53 v2 pre-gmime-3.0 cleanup David Bremner
2017-05-23  0:53 ` [Patch v2 1/7] test/multipart: reorganize creation of multipart message David Bremner
2017-05-23  0:53 ` [Patch v2 2/7] test: mark inclusion of headers with raw body as bug David Bremner
2017-05-23  0:53 ` [Patch v2 3/7] util: convenience function to create gmime stream for stdout David Bremner
2017-05-23  0:53 ` [Patch v2 4/7] cli/reply: direct all output for text format to gmime stream David Bremner
2017-05-23  0:53 ` [Patch v2 5/7] cli/show: use single stream for printf / gmime object output David Bremner
2017-05-23  0:53 ` [Patch v2 6/7] perf-test: add memory test for reply David Bremner
2017-05-23  0:53 ` [Patch v2 7/7] cli/reply: fix two memory leaks, document a third David Bremner
2017-05-24  0:50   ` [PATCH] test/thread-naming: remove excess escaping from sender address David Bremner
2017-05-24 23:53     ` [PATCH] test: test parsing of malformed from addresses David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).