git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Jeff King <peff@peff.net>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 1/1] vreportf(): avoid relying on stdio buffering
Date: Tue, 29 Oct 2019 15:21:37 +0100	[thread overview]
Message-ID: <1a6119b4-b44f-0edd-da18-46b7d31a274f@syntevo.com> (raw)
In-Reply-To: <e426627e1494e31e548fe044c1c1806ff59340cf.1572356272.git.gitgitgadget@gmail.com>

On 29.10.2019 14:37, Johannes Schindelin via GitGitGadget wrote:

 > -	vsnprintf(msg, sizeof(msg), err, params);
 > -	for (p = msg; *p; p++) {
 > +	p = msg + off < pend ? msg + off : pend - 1;

According to my understanding, this is undefined behavior:

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3242.pdf
... Unless both pointers point to elements of the same array object, or 
one past the last element of the array object, the behavior is undefined.

 > The MSVC runtime behavior differs from glibc's with respect to
 > `fprintf(stderr, ...)` in that the former writes out the message
 > character by character.

Can you please preserve the research text about fprintf() behavior on 
different platforms that I provided before? Change formatting to what 
you think fits best.

fprintf() has problems with any buffering settings
--------------------------------------------------
1) If `stderr` is fully buffered:
    the ordering of `stdout` and `stderr` messages could be wrong,
    because `stderr` output waits for the buffer to become full.
2) If `stderr` has any type of buffering:
    buffer has fixed size, which could lead to interleaved buffer blocks
    when two threads/processes write at the same time.
3) If `stderr` is not buffered:
    Some implementations, such as VC++ and MinGW, literally disable
    buffering and `fprintf()` will output char-by-char, which leads to
    unreadable char-interleaved writes if two processes write to
    `stderr` at the same time (threads are OK because `fprintf()`
    usually locks `FILE*` in current process).
4) If stderr is line buffered: MinGW/VC++ will enable full buffering
    instead. See MSDN for `setvbuf()`.

fprintf() behavior in git, per platform
---------------------------------------
1) libc - large outputs can be block-interleaved
    fprintf() enables temporary stream buffering.
    Code references:
        buffered_vfprintf()
2) VC++ - char-interleaved
    fprintf() enables temporary stream buffering, but only if stream was
    not set to no buffering. This has no effect, because stderr is not
    buffered by default, and git takes an extra step to ensure that in
    `swap_osfhnd()`.
    Code references:
        _iob[_IOB_ENTRIES]
        __acrt_stdio_temporary_buffering_guard
        has_any_buffer()
3) MinGW - char-interleaved (console), full buffering (file)
    `fprintf()` obeys `stderr` buffering. But it uses old MSVCRT.DLL,
    which eventually calls `_flsbuf()`, which enables buffering unless
    `isatty(stderr)` or buffering is disabled. Buffering is not disabled
    by default for `stderr`. Therefore, buffering is enabled only for
    file-redirected `stderr`.
    Code references:
        __mingw_vfprintf()
        __pformat_wcputs()
        _fputc_nolock()
        _flsbuf()
        _iob[_IOB_ENTRIES]


  reply	other threads:[~2019-10-29 14:21 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-28 15:00 [PATCH 0/1] Fix t5516 flakiness in Visual Studio builds Johannes Schindelin via GitGitGadget
2019-10-28 15:00 ` [PATCH 1/1] vreportf(): avoid buffered write in favor of unbuffered one Johannes Schindelin via GitGitGadget
2019-10-29  3:18   ` Junio C Hamano
2019-10-29 12:30     ` Johannes Schindelin
2019-10-29 13:49       ` Jeff King
2019-10-29 14:13         ` Johannes Schindelin
2019-10-29 14:32           ` Jeff King
2019-10-29 20:09             ` Johannes Schindelin
2019-10-30  1:43               ` Junio C Hamano
2019-10-29 16:44         ` Junio C Hamano
2019-10-29 10:38   ` SZEDER Gábor
2019-10-29 12:38     ` Johannes Schindelin
2019-10-29 13:52       ` Jeff King
2019-10-29 14:18         ` Johannes Schindelin
2019-10-29 13:37 ` [PATCH v2 0/1] Fix t5516 flakiness in Visual Studio builds Johannes Schindelin via GitGitGadget
2019-10-29 13:37   ` [PATCH v2 1/1] vreportf(): avoid relying on stdio buffering Johannes Schindelin via GitGitGadget
2019-10-29 14:21     ` Alexandr Miloslavskiy [this message]
2019-10-29 19:57       ` Johannes Schindelin
2019-10-29 20:09         ` Jeff King
2019-10-29 20:24           ` Alexandr Miloslavskiy
2019-10-29 20:11         ` Alexandr Miloslavskiy
2019-10-29 20:01   ` [PATCH v3 0/1] Fix t5516 flakiness in Visual Studio builds Johannes Schindelin via GitGitGadget
2019-10-29 20:01     ` [PATCH v3 1/1] vreportf(): avoid relying on stdio buffering Johannes Schindelin via GitGitGadget
2019-10-29 20:32       ` Jeff King
2019-10-30  8:54         ` Johannes Schindelin
2019-10-31  6:24           ` Jeff King
2019-10-31 10:26             ` Johannes Schindelin
2019-10-31 15:48               ` Jeff King
2019-11-01 18:41                 ` Johannes Schindelin
2019-10-30  2:01       ` Junio C Hamano
2019-10-30  9:13         ` Johannes Schindelin
2019-10-30 10:44     ` [PATCH v4 0/1] Fix t5516 flakiness in Visual Studio builds Johannes Schindelin via GitGitGadget
2019-10-30 10:44       ` [PATCH v4 1/1] vreportf(): avoid relying on stdio buffering Johannes Schindelin via GitGitGadget
2019-11-02  4:05         ` 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=1a6119b4-b44f-0edd-da18-46b7d31a274f@syntevo.com \
    --to=alexandr.miloslavskiy@syntevo.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=peff@peff.net \
    --subject='Re: [PATCH v2 1/1] vreportf(): avoid relying on stdio buffering' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).