Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> On Fri, Mar 31, 2017 at 5:52 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>>
>> The continuation logic is oddly complex, and I can't follow the logic.
>> But it is completely broken in how it thinks empty lines are somehow
>> "continuations".
>
> The attached patch seems to work for me. Comments?
We start at header_stage set to 1, keep skipping empty lines while
in that state, and we stay in that state as long as we see in-body
header (or a continuation of the in-body header we saw earlier). We
get out of this state when we see a blank line after we are done
with the in-body headers. Once header_stage is set to 0 with a
blank line, we don't do in-body headers (scissors will roll back the
whole thing and irrelevant to the analysis of correctness).
But you found that "keep skipping empty" done unconditionally is
wrong, because we may have already seen an in-body header and are
trying to see if the line is a continuation (in which case
check_inbody_header() would append to the previous) or another
in-body header (in which case again check_inbody_header() would use
it), or something else (in which case check_inbody_header() will
return false, we get out of header_stage=1 state and process this
line as the first line of the log message. An empty line we see in
this state must trigger "we are no longer taking in-body header"
logic, too.
And that is exactly your patch does. The change "feels" correct to
me.
> mailinfo.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/mailinfo.c b/mailinfo.c
> index a489d9d0f..68037758f 100644
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -757,8 +757,13 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)
> assert(!mi->filter_stage);
>
> if (mi->header_stage) {
> - if (!line->len || (line->len == 1 && line->buf[0] == '\n'))
> + if (!line->len || (line->len == 1 && line->buf[0] == '\n')) {
> + if (mi->inbody_header_accum.len) {
> + flush_inbody_header_accum(mi);
> + mi->header_stage = 0;
> + }
> return 0;
> + }
> }
>
> if (mi->use_inbody_headers && mi->header_stage) {