--- /dev/null
+Return-Path: <amdragon@mit.edu>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id BC6CC431FAF\r
+ for <notmuch@notmuchmail.org>; Wed, 18 Jan 2012 18:24:07 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id CTxMdQQHg9Mg for <notmuch@notmuchmail.org>;\r
+ Wed, 18 Jan 2012 18:24:07 -0800 (PST)\r
+Received: from dmz-mailsec-scanner-8.mit.edu (DMZ-MAILSEC-SCANNER-8.MIT.EDU\r
+ [18.7.68.37])\r
+ by olra.theworths.org (Postfix) with ESMTP id F307D431FAE\r
+ for <notmuch@notmuchmail.org>; Wed, 18 Jan 2012 18:24:06 -0800 (PST)\r
+X-AuditID: 12074425-b7f4a6d0000008e0-ac-4f177ec508ca\r
+Received: from mailhub-auth-3.mit.edu ( [18.9.21.43])\r
+ by dmz-mailsec-scanner-8.mit.edu (Symantec Messaging Gateway) with SMTP\r
+ id EF.DA.02272.5CE771F4; Wed, 18 Jan 2012 21:24:05 -0500 (EST)\r
+Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
+ by mailhub-auth-3.mit.edu (8.13.8/8.9.2) with ESMTP id q0J2O4Xu023561; \r
+ Wed, 18 Jan 2012 21:24:04 -0500\r
+Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
+ (authenticated bits=0)\r
+ (User authenticated as amdragon@ATHENA.MIT.EDU)\r
+ by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q0J2O3Ie004223\r
+ (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
+ Wed, 18 Jan 2012 21:24:04 -0500 (EST)\r
+Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
+ (envelope-from <amdragon@MIT.EDU>)\r
+ id 1RnhfI-00028F-2x; Wed, 18 Jan 2012 21:23:48 -0500\r
+Date: Wed, 18 Jan 2012 21:23:48 -0500\r
+From: Austin Clements <amdragon@MIT.EDU>\r
+To: Jani Nikula <jani@nikula.org>\r
+Subject: Re: [PATCH 3/3] show: Introduce mime_node formatter callback\r
+Message-ID: <20120119022348.GK16740@mit.edu>\r
+References: <1326918507-28033-1-git-send-email-amdragon@mit.edu>\r
+ <1326918507-28033-4-git-send-email-amdragon@mit.edu>\r
+ <87wr8obpyv.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain; charset=us-ascii\r
+Content-Disposition: inline\r
+In-Reply-To: <87wr8obpyv.fsf@nikula.org>\r
+User-Agent: Mutt/1.5.21 (2010-09-15)\r
+X-Brightmail-Tracker:\r
+ H4sIAAAAAAAAA+NgFprHKsWRmVeSWpSXmKPExsUixCmqrXu0TtzfoP2xpkVr92cmi6tb+9kt\r
+ mqY7W+zZ52Vx/eZMZgdWj7Pd7awed09zeeycdZfd49b91+wez1bdYg5gjeKySUnNySxLLdK3\r
+ S+DK6P/Wxl6wRaXi4vufTA2Me6S7GDk5JARMJG4/X8oGYYtJXLi3Hsjm4hAS2McosazlLQuE\r
+ s4FR4syGXlYI5ySTxNYXpxghnCWMEutv3mMC6WcRUJVoaZ/EDmKzCWhIbNu/nBHEFhFQlNh8\r
+ cj+QzcHBLFAs0fOxCsQUFnCVWPtBFaSCV0BHYs+clewQI6cyStz4d4INIiEocXLmExYQm1lA\r
+ Cyj+kglijLTE8n8cIGFOoE0bvr9kBrFFBVQkppzcxjaBUWgWku5ZSLpnIXQvYGRexSibklul\r
+ m5uYmVOcmqxbnJyYl5dapGuhl5tZopeaUrqJERQN7C6qOxgnHFI6xCjAwajEwxshIu4vxJpY\r
+ VlyZe4hRkoNJSZT3QDVQiC8pP6UyI7E4I76oNCe1+BCjBAezkgjvThOgHG9KYmVValE+TEqa\r
+ g0VJnFdT652fkEB6YklqdmpqQWoRTFaGg0NJgreiBqhRsCg1PbUiLTOnBCHNxMEJMpwHaPgd\r
+ kMW8xQWJucWZ6RD5U4yKUuK8VsB0IyQAksgozYPrhSWrV4ziQK8I856oBariASY6uO5XQIOZ\r
+ gAZ7NImBDC5JREhJNTAuzl3vmiz8z0bR/ZZsbfLKsg2v/+8LV9J36zOMnvX/e9eOBY2Tjefx\r
+ SM5+9HHi+Zm76iVtwwrnVUVuP1F8SnE/g+2vV1MVpynPX3Wc81KB+AfXQxOvdHDqpmd7RaVN\r
+ 1yu08RXXtv5rZvlto6asj9hsV1vFFKuPAWdWsk6edvNNbo1Ajeo8179KLMUZiYZazEXFiQDQ\r
+ aoPjMQMAAA==\r
+Cc: notmuch@notmuchmail.org, dkg@fifthhorseman.net\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Thu, 19 Jan 2012 02:24:07 -0000\r
+\r
+Quoth Jani Nikula on Jan 19 at 12:33 am:\r
+> On Wed, 18 Jan 2012 15:28:27 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
+> > This callback is the gateway to the new mime_node_t-based formatters.\r
+> > This maintains backwards compatibility so the formatters can be\r
+> > transitioned one at a time. Once all formatters are converted, the\r
+> > formatter structure can be reduced to only message_set_{start,sep,end}\r
+> > and part, most of show_message can be deleted, and all of\r
+> > show-message.c can be deleted.\r
+> > ---\r
+> > notmuch-client.h | 6 ++++++\r
+> > notmuch-reply.c | 2 +-\r
+> > notmuch-show.c | 22 ++++++++++++++++++----\r
+> > 3 files changed, 25 insertions(+), 5 deletions(-)\r
+> > \r
+> > diff --git a/notmuch-client.h b/notmuch-client.h\r
+> > index b3dcb6b..3ccdfad 100644\r
+> > --- a/notmuch-client.h\r
+> > +++ b/notmuch-client.h\r
+> > @@ -54,8 +54,14 @@\r
+> > #define STRINGIFY(s) STRINGIFY_(s)\r
+> > #define STRINGIFY_(s) #s\r
+> > \r
+> > +struct mime_node;\r
+> > +struct notmuch_show_params;\r
+> > +\r
+> > typedef struct notmuch_show_format {\r
+> > const char *message_set_start;\r
+> > + void (*part) (const void *ctx,\r
+> > + struct mime_node *node, int indent,\r
+> > + struct notmuch_show_params *params);\r
+> > const char *message_start;\r
+> > void (*message) (const void *ctx,\r
+> > notmuch_message_t *message,\r
+> > diff --git a/notmuch-reply.c b/notmuch-reply.c\r
+> > index 0f682db..9a224e2 100644\r
+> > --- a/notmuch-reply.c\r
+> > +++ b/notmuch-reply.c\r
+> > @@ -31,7 +31,7 @@ static void\r
+> > reply_part_content (GMimeObject *part);\r
+> > \r
+> > static const notmuch_show_format_t format_reply = {\r
+> > - "",\r
+> > + "", NULL,\r
+> > "", NULL,\r
+> > "", NULL, reply_headers_message_part, ">\n",\r
+> > "",\r
+> > diff --git a/notmuch-show.c b/notmuch-show.c\r
+> > index ecadfa8..46eef44 100644\r
+> > --- a/notmuch-show.c\r
+> > +++ b/notmuch-show.c\r
+> > @@ -42,7 +42,7 @@ static void\r
+> > format_part_end_text (GMimeObject *part);\r
+> > \r
+> > static const notmuch_show_format_t format_text = {\r
+> > - "",\r
+> > + "", NULL,\r
+> > "\fmessage{ ", format_message_text,\r
+> > "\fheader{\n", format_headers_text, format_headers_message_part_text, "\fheader}\n",\r
+> > "\fbody{\n",\r
+> > @@ -85,7 +85,7 @@ static void\r
+> > format_part_end_json (GMimeObject *part);\r
+> > \r
+> > static const notmuch_show_format_t format_json = {\r
+> > - "[",\r
+> > + "[", NULL,\r
+> > "{", format_message_json,\r
+> > "\"headers\": {", format_headers_json, format_headers_message_part_json, "}",\r
+> > ", \"body\": [",\r
+> > @@ -106,7 +106,7 @@ format_message_mbox (const void *ctx,\r
+> > unused (int indent));\r
+> > \r
+> > static const notmuch_show_format_t format_mbox = {\r
+> > - "",\r
+> > + "", NULL,\r
+> > "", format_message_mbox,\r
+> > "", NULL, NULL, "",\r
+> > "",\r
+> > @@ -125,7 +125,7 @@ static void\r
+> > format_part_content_raw (GMimeObject *part);\r
+> > \r
+> > static const notmuch_show_format_t format_raw = {\r
+> > - "",\r
+> > + "", NULL,\r
+> > "", NULL,\r
+> > "", NULL, format_headers_message_part_text, "\n",\r
+> > "",\r
+> > @@ -762,6 +762,20 @@ show_message (void *ctx,\r
+> > int indent,\r
+> > notmuch_show_params_t *params)\r
+> > {\r
+> > + if (format->part) {\r
+> > + void *local = talloc_new (ctx);\r
+> > + mime_node_t *root, *part;\r
+> > +\r
+> > + if (mime_node_open (local, message, params->cryptoctx, params->decrypt,\r
+> > + &root) != NOTMUCH_STATUS_SUCCESS)\r
+> \r
+> I'm new to talloc, I think I like it, but I always find it confusing\r
+> when some code paths free the contexts, and some don't. Like here.\r
+> \r
+> Are you not freeing the local context here because it's just an empty\r
+> context, and freeing below because it's no longer empty?\r
+\r
+No, that's just a bug. In practice it probably doesn't matter much\r
+because, as you pointed out, the local context consumes very little\r
+memory and since the caller will eventually free ctx, local will get\r
+cleaned up, too. So, while this isn't strictly a memory leak, in\r
+principle this could add up before ctx gets freed. Fixed.\r
+\r
+> No need to change anything, I guess, just asking...\r
+\r
+I wait a little for other comments and then send a new version.\r
+\r
+Thanks for the review!\r
+\r
+> BR,\r
+> Jani.\r
+> \r
+> > + return;\r
+> > + part = mime_node_seek_dfs (root, params->part < 0 ? 0 : params->part);\r
+> > + if (part)\r
+> > + format->part (local, part, indent, params);\r
+> > + talloc_free (local);\r
+> > + return;\r
+> > + }\r
+> > +\r
+> > if (params->part <= 0) {\r
+> > fputs (format->message_start, stdout);\r
+> > if (format->message)\r
projects / notmuch-archives.git / commitdiff