On Thu, Apr 07, 2016 at 10:08:37AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" <m...@redhat.com> writes:
>
> > Slightly slower, but will allow easy additional processing on it.
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
>
> I haven't read 4/4 yet, but can guess from what this patch does that
> the next step would let others futz with the contents of the message
> that is on disk (i.e. what mailinfo() wrote out, which is identical
> to what we have in mi.log_message at this point of the codeflow)
> before you do the new strbuf_read_file().
>
> It probably is better to do this as part of 4/4; it is easier to
> understand why this is a good and necessary thing to do. An obvious
> improvement is to omit this extra "read back from the filesystem"
> when we won't be making any interpret-trailer calls (i.e. no -t
> option from the command line), but if we stop at this step 3/4, then
> we'd end up wasting cycles without having any benefit.
Hmm - splitting it out was easy for development since I could verify all
tests pass. But if you do want the optimization, then sure, I'll have
to squash it in.
> > builtin/am.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/am.c b/builtin/am.c
> > index d003939..4180b04 100644
> > --- a/builtin/am.c
> > +++ b/builtin/am.c
> > @@ -1246,6 +1246,7 @@ static int parse_mail(struct am_state *state, const
> > char *mail)
> > FILE *fp;
> > struct strbuf sb = STRBUF_INIT;
> > struct strbuf msg = STRBUF_INIT;
> > + struct strbuf log_msg = STRBUF_INIT;
> > struct strbuf author_name = STRBUF_INIT;
> > struct strbuf author_date = STRBUF_INIT;
> > struct strbuf author_email = STRBUF_INIT;
> > @@ -1330,7 +1331,12 @@ static int parse_mail(struct am_state *state, const
> > char *mail)
> > }
> >
> > strbuf_addstr(&msg, "\n\n");
> > - strbuf_addbuf(&msg, &mi.log_message);
> > +
> > + if (strbuf_read_file(&log_msg, am_path(state, "msg"), 0) < 0) {
> > + die_errno(_("could not read '%s'"), am_path(state, "msg"));
> > + }
>
> I do not think these {} serve any purpose; drop them?
>
> > +
> > + strbuf_addbuf(&msg, &log_msg);
> > strbuf_stripspace(&msg, 0);
> >
> > if (state->signoff)
> > @@ -1349,6 +1355,7 @@ static int parse_mail(struct am_state *state, const
> > char *mail)
> > state->msg = strbuf_detach(&msg, &state->msg_len);
> >
> > finish:
> > + strbuf_release(&log_msg);
> > strbuf_release(&msg);
> > strbuf_release(&author_date);
> > strbuf_release(&author_email);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
