v2 pre-gmime-3.0 cleanup

classic Classic list List threaded Threaded
10 messages Options
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

v2 pre-gmime-3.0 cleanup

this obsoletes the series at

     id:[hidden email]

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:[hidden email]

and confirmed with gmime upstream, that behaviour can be considered a
bug in gmime-2.6, and goes away in gmime-3.0
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[Patch v2 1/7] test/multipart: reorganize creation of multipart message

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 <[hidden email]>
-To: [hidden email]
-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: <[hidden email]>
-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 <[hidden email]>
 To: [hidden email]
-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: <[hidden email]>
+Message-ID: <[hidden email]>
 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 <[hidden email]>
+To: [hidden email]
+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: <[hidden email]>
+MIME-Version: 1.0
+EOF
+
+cat multipart_body >> ${MAIL_DIR}/multipart
+
 cat <<EOF > ${MAIL_DIR}/base64-part-with-crlf
 From: Carl Worth <[hidden email]>
 To: [hidden email]
--
2.11.0

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[Patch v2 2/7] test: mark inclusion of headers with raw body as bug

In reply to this post by David Bremner-2
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:[hidden email]' >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:[hidden email]' >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:[hidden email]' >OUTPUT
@@ -518,30 +519,9 @@ notmuch show --format=raw --part=3 'id:[hidden email]' >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:[hidden email]' >OUTPUT
-cat <<EOF >EXPECTED
-From: Carl Worth <[hidden email]>
-To: [hidden email]
-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: <[hidden email]>
-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:[hidden email]' >OUTPUT
--
2.11.0

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[Patch v2 3/7] util: convenience function to create gmime stream for stdout

In reply to this post by David Bremner-2
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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[Patch v2 4/7] cli/reply: direct all output for text format to gmime stream

In reply to this post by David Bremner-2
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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[Patch v2 5/7] cli/show: use single stream for printf / gmime object output

In reply to this post by David Bremner-2
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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[Patch v2 6/7] perf-test: add memory test for reply

In reply to this post by David Bremner-2
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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[Patch v2 7/7] cli/reply: fix two memory leaks, document a third

In reply to this post by David Bremner-2
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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[PATCH] test/thread-naming: remove excess escaping from sender address.

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 \<[hidden email]\>"'
+            '[from]="Empty Sender <[hidden email]>"'
 empty_parent=${gen_msg_id}
 add_message '[subject]="@FORCE_EMPTY"' \
     '[date]="Sun, 14 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<[hidden email]\>"' \
+            '[from]="Empty Sender <[hidden email]>"' \
             "[in-reply-to]=\<$empty_parent\>"
 output=$(notmuch search --sort=newest-first from:[hidden email] | 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 \<[hidden email]\>"' \
+            '[from]="Empty Sender <[hidden email]>"' \
             "[in-reply-to]=\<$empty_parent\>"
 add_message '[subject]="Non-empty subject (2)"' \
     '[date]="Mon, 16 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<[hidden email]\>"' \
+            '[from]="Empty Sender <[hidden email]>"' \
             "[in-reply-to]=\<$empty_parent\>"
 add_message '[subject]="@FORCE_EMPTY"' \
     '[date]="Tue, 17 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<[hidden email]\>"' \
+            '[from]="Empty Sender <[hidden email]>"' \
             "[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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

[PATCH] test: test parsing of malformed from addresses

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 \<[hidden email]\>"'
+output=$(notmuch search --sort=newest-first from:[hidden email] | 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

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch