All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org,
	Kirill Likhodedov <Kirill.Likhodedov@jetbrains.com>,
	Johannes Sixt <j.sixt@viscovery.net>,
	Erik Faye-Lund <kusmabite@gmail.com>
Subject: Re: [PATCH] rev-list: handle %x00 NUL in user format
Date: Wed, 13 Oct 2010 20:38:54 -0400	[thread overview]
Message-ID: <20101014003853.GA10708@coredump.intra.peff.net> (raw)
In-Reply-To: <7vtykpiwkv.fsf@alter.siamese.dyndns.org>

On Wed, Oct 13, 2010 at 02:58:40PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Erik mentioned a potential problem with fwrite() and the way we handle
> > ANSI emulation for Windows. I think if there is a problem, then the same
> > problem exists in the --graph code, and we should do this, and then fix
> > both on top.
> 
> I am quite sure that --graph codepath is used a lot less often than this
> codepath, and most users do not hit the problem you are fixing---that is
> why it took so long to get noticed.  I think this printf() has been there
> from the beginning, even though the possibility of placing a NUL in
> buf.buf may have come much later, like when %x00 was introduced).
> 
> Use of fwrite() here will affect all Windows users who use color (probably
> most of them), no?  It may be a deliberate regression, but it has a real
> cost associated with it; it will break more usual uses on Windows to help
> an obscure use case on POSIX.

Yeah, I don't want to regress Windows users in an actual release. I was
hoping for you to queue it on a side branch, and then have Windows
people apply the proper fix on top before getting it merged in.

-Peff

      reply	other threads:[~2010-10-14  0:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-07 18:25 [PATCH] rev-list: handle %x00 NUL in user format Jeff King
2010-10-13 21:58 ` Junio C Hamano
2010-10-14  0:38   ` Jeff King [this message]

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=20101014003853.GA10708@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=Kirill.Likhodedov@jetbrains.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.net \
    --cc=kusmabite@gmail.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.