From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
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 14:58:40 -0700 [thread overview]
Message-ID: <7vtykpiwkv.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: 20101007182542.GA20165@sigill.intra.peff.net
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.
As to emulating fwrite(3) on Windows, even though there are a lot of
callers of fwrite(3) that do not want the payload colored, it is probably
sufficient to check isatty(fileno(stream)) upfront like all the other
winansi_fEMULATED() functions do, and to make ansi_emulate() call the
system fwrite(3) directly.
I'll queue this patch on a side branch and I may even feel like merging it
to 'next', but we would need to hear from Windows folks how they want to
proceed soon.
next prev parent reply other threads:[~2010-10-13 21:59 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 [this message]
2010-10-14 0:38 ` Jeff King
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=7vtykpiwkv.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=Kirill.Likhodedov@jetbrains.com \
--cc=git@vger.kernel.org \
--cc=j.sixt@viscovery.net \
--cc=kusmabite@gmail.com \
--cc=peff@peff.net \
/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.