public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Jerzy Kozera <jerzy.kozera@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] gpg-interface: Strip CR chars for Windows gpg2
Date: Sun, 12 Nov 2017 10:51:33 +0900	[thread overview]
Message-ID: <xmqqr2t4cli2.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171111160657.11016-1-jerzy.kozera@gmail.com> (Jerzy Kozera's message of "Sat, 11 Nov 2017 16:06:57 +0000")

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

> +/* 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.

next prev parent reply	other threads:[~2017-11-12  1:51 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-11 16:06 [PATCH] gpg-interface: Strip CR chars for Windows gpg2 Jerzy Kozera
2017-11-12  1:51 ` Junio C Hamano [this message]
2017-11-12  1:53 ` Junio C Hamano
2017-11-12  5:58 ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=xmqqr2t4cli2.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=jerzy.kozera@gmail$(echo .)com \
    /path/to/YOUR_REPLY

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox