Re: [PATCH v2 2/5] fast-import: don't fail on omitted committer name

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 11, 2011, 22:06 UTC
Message-ID: <7vwrejppq4.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1313058070-4774-3-git-send-email-divanorama@gmail.com>

Dmitry Ivankov <divanorama@gmail.com> writes:

Show 12 quoted lines

> diff --git a/fast-import.c b/fast-import.c
> index 7cc2262..ed1f7c9 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1973,6 +1973,10 @@ static char *parse_ident(const char *buf)
>  	size_t name_len;
>  	char *ident;
>  
> +	/* ensure there is a space delimiter even if there is no name */
> +	if (*buf == '<')
> +		--buf;
> +

This is somewhat cryptic, even though it may be correct, especially if the reader of the code does not know that this function is called by the caller after reading "author " (or committer/tagger) and buf points at one byte beyond that SP after the string that specifies the kind of the person, hence "--buf" [*1*] makes the subsequent strncpy() start copying from that SP which makes the result correct.

Perhaps an additional comment before the function is in order?

Thanks.

[Footnote]

*1* by the way, as pure style thing, I think our codebase favors to use post-*crement, i.e. "buf--", if you are doing pre/post-*crement purely for its side effect.