Re: [PATCH] gpg-interface: Strip CR chars for Windows gpg2

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 12, 2017, 01:51 UTC
Message-ID: <xmqqr2t4cli2.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171111160657.11016-1-jerzy.kozera@gmail.com>

Jerzy Kozera <jerzy.kozera@gmail.com> writes:

Show 12 quoted lines

> +/* Strip CR from the line endings, in case we are on Windows. */
> +static void strip_cr(struct strbuf *buffer, size_t bottom) {
> +	size_t i, j;
> +	for (i = j = bottom; i < buffer->len; i++)
> +		if (buffer->buf[i] != '\r') {
> +			if (i != j)
> +				buffer->buf[j] = buffer->buf[i];
> +			j++;
> +		}
> +	strbuf_setlen(buffer, j);
> +}
> +

While the approach of turning CRLF into LF as a workaround is a good one, I do not see this loop doing that; instead it is removing CR anywhere on the line unconditionally.

It might not make a difference in practice to rely on the assumption that nobody sane would place a lone CR in the middle of a line, but it feels dirty.

Because I know this was lifted from an existing code, I think it may be better to take this patch as-is, but the code screams loudly that it wants a clean-up patch to fix that immediately on top of it.

Thanks.