unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
To: Austin Clements <amdragon@MIT.EDU>
Cc: notmuch@notmuchmail.org
Subject: Re: [PATCH 3/4] Utility function to seek in MIME trees in depth-first order.
Date: Mon, 26 Dec 2011 03:39:57 +0400	[thread overview]
Message-ID: <87pqfc1ar6.fsf@gmail.com> (raw)
In-Reply-To: <20111224034603.GC1927@mit.edu>

On Fri, 23 Dec 2011 22:46:19 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Dmitry Kurochkin on Dec 10 at  3:43 pm:
> > On Fri,  9 Dec 2011 14:54:27 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > > This function matches how we number parts for the --part argument to
> > > show.  It will allow us to jump directly to the desired part, rather
> > > than traversing the entire tree and carefully tracking whether or not
> > > we're "in the zone".
> > > ---
> > >  mime-node.c      |   25 +++++++++++++++++++++++++
> > >  notmuch-client.h |    5 +++++
> > >  2 files changed, 30 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/mime-node.c b/mime-node.c
> > > index a8e4a59..207818e 100644
> > > --- a/mime-node.c
> > > +++ b/mime-node.c
> > > @@ -232,3 +232,28 @@ mime_node_child (const mime_node_t *parent, int child)
> > >  			g_type_name (G_OBJECT_TYPE (parent->part)));
> > >      }
> > >  }
> > > +
> > > +static mime_node_t *
> > > +_mime_node_seek_dfs_walk (mime_node_t *node, int *n)
> > > +{
> > > +    mime_node_t *ret = NULL;
> > > +    int i;
> > > +
> > 
> > Can we move declarations below the if (which does not need them)?  I
> > always have troubles remembering if (recent enough) C standard allows
> > that or it is a GCC extension.  FWIW in the previous patch there are
> > declarations in the middle of a block, e.g.:
> > 
> > 	} else {
> > 	    out->is_signed = TRUE;
> >             ...
> > 	    GMimeSignatureValidity *sig_validity = g_mime_multipart_signed_verify
> > 		(GMIME_MULTIPART_SIGNED (part), out->ctx->cryptoctx, &err);
> > 
> > So either we can move these declarations to where they are needed, or we
> > should fix it in _mime_node_create().
> 
> Since prevailing notmuch style seems to be top-declarations, I fixed
> up _mime_node_create instead (personally I prefer C99-style
> declarations, but *shrug*).
> 

If there is any code which already uses C99-style declarations, then we
should use them in the new code IMO.  Perhaps whether to use C99-style
declarations or not should be a coding style requirement.

Regards,
  Dmitry

> > > +    if (*n <= 0)
> > 
> > Comment for mime_node_seek_dfs() says that the function returns the node
> > itself for n = 0, but does not say anything about n < 0.  I would expect
> > the function to return NULL for n < 0.  In any case, the comment below
> > should probably mention what happens for n < 0;
> 
> Good point.  I made it return NULL for n < 0.  I think this logically
> falls under "Returns NULL if there is no such part."
> 
> > > +	return node;
> > > +
> > > +    *n = *n - 1;
> > 
> > Perhaps *n -= 1?  Or even --(*n)?
> 
> Changed to *n -= 1.
> 
> > > +    for (i = 0; i < node->children && !ret; i++) {
> > 
> > Consider s/i++/++i/.
> 
> notmuch uses i++ remarkably consistently, so I left this.
> 
> > Regards,
> >   Dmitry

  reply	other threads:[~2011-12-25 23:40 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-28  2:21 [PATCH 0/4] First step of 'show' rewrite Austin Clements
2011-11-28  2:21 ` [PATCH 1/4] show: Pass notmuch_message_t instead of path to show_message_body Austin Clements
2011-11-28  2:21 ` [PATCH 2/4] Introduce a generic tree-like abstraction for MIME traversal Austin Clements
2011-11-29 19:11   ` Jani Nikula
2011-12-04 19:26     ` Austin Clements
2011-11-28  2:21 ` [PATCH 3/4] Utility function to seek in MIME trees in depth-first order Austin Clements
2011-11-28  2:21 ` [PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface Austin Clements
2011-11-28 19:44   ` Jani Nikula
2011-11-29 14:37 ` [PATCH 0/4] First step of 'show' rewrite Jameson Graef Rollins
2011-12-04 19:31 ` [PATCH v2 " Austin Clements
2011-12-04 19:31   ` [PATCH 1/4] show: Pass notmuch_message_t instead of path to show_message_body Austin Clements
2011-12-09 19:05     ` Dmitry Kurochkin
2011-12-09 19:54       ` Austin Clements
2011-12-09 19:59         ` Dmitry Kurochkin
2011-12-04 19:31   ` [PATCH 2/4] Introduce a generic tree-like abstraction for MIME traversal Austin Clements
2011-12-04 19:31   ` [PATCH 3/4] Utility function to seek in MIME trees in depth-first order Austin Clements
2011-12-04 19:31   ` [PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface Austin Clements
2011-12-09 17:39   ` [PATCH v2 0/4] First step of 'show' rewrite Austin Clements
2011-12-09 17:40     ` Dmitry Kurochkin
2011-12-09 17:51       ` Jameson Graef Rollins
2011-12-09 19:54   ` [PATCH v3 " Austin Clements
2011-12-09 19:54     ` [PATCH 1/4] show: Pass notmuch_message_t instead of path to show_message_body Austin Clements
2011-12-09 19:54     ` [PATCH 2/4] Introduce a generic tree-like abstraction for MIME traversal Austin Clements
2011-12-09 23:25       ` Dmitry Kurochkin
2011-12-10 21:17         ` Jameson Graef Rollins
2011-12-24  3:45           ` Austin Clements
2011-12-24  3:45         ` Austin Clements
2011-12-27 14:27           ` Daniel Kahn Gillmor
2011-12-28  3:23             ` Austin Clements
2011-12-10 21:19       ` Jameson Graef Rollins
2011-12-09 19:54     ` [PATCH 3/4] Utility function to seek in MIME trees in depth-first order Austin Clements
2011-12-10 11:43       ` Dmitry Kurochkin
2011-12-24  3:46         ` Austin Clements
2011-12-25 23:39           ` Dmitry Kurochkin [this message]
2011-12-09 19:54     ` [PATCH 4/4] show: Rewrite show_message_body to use the MIME tree interface Austin Clements
2011-12-11 10:34       ` Dmitry Kurochkin
2011-12-24  3:46         ` Austin Clements
2011-12-10 21:16     ` [PATCH v3 0/4] First step of 'show' rewrite Jameson Graef Rollins
2011-12-11 10:41     ` Dmitry Kurochkin
2011-12-15 11:03       ` David Bremner
2011-12-24  3:45     ` [PATCH v4 " Austin Clements
2011-12-24  3:45       ` [PATCH v4 1/4] show: Pass notmuch_message_t instead of path to show_message_body Austin Clements
2011-12-24  3:45       ` [PATCH v4 2/4] Introduce a generic tree-like abstraction for MIME traversal Austin Clements
2011-12-24  7:55         ` Jameson Graef Rollins
2011-12-24  8:05           ` Dmitry Kurochkin
2011-12-24 19:03           ` Austin Clements
2011-12-24  3:45       ` [PATCH v4 3/4] Utility function to seek in MIME trees in depth-first order Austin Clements
2011-12-24  3:45       ` [PATCH v4 4/4] show: Rewrite show_message_body to use the MIME tree interface Austin Clements
2011-12-24  3:58       ` [PATCH v4 0/4] First step of 'show' rewrite Dmitry Kurochkin
2011-12-24  7:55       ` Jameson Graef Rollins
2011-12-24 18:52       ` [PATCH v5 " Austin Clements
2011-12-24 18:52         ` [PATCH v5 1/4] show: Pass notmuch_message_t instead of path to show_message_body Austin Clements
2011-12-24 18:52         ` [PATCH v5 2/4] Introduce a generic tree-like abstraction for MIME traversal Austin Clements
2011-12-24 18:52         ` [PATCH v5 3/4] Utility function to seek in MIME trees in depth-first order Austin Clements
2011-12-24 18:52         ` [PATCH v5 4/4] show: Rewrite show_message_body to use the MIME tree interface Austin Clements
2011-12-25 23:35         ` [PATCH v5 0/4] First step of 'show' rewrite Dmitry Kurochkin
2011-12-26  2:42         ` David Bremner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://notmuchmail.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pqfc1ar6.fsf@gmail.com \
    --to=dmitry.kurochkin@gmail.com \
    --cc=amdragon@MIT.EDU \
    --cc=notmuch@notmuchmail.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).