Hi Alex,

On Tue, 29 Oct 2019, Alexandr Miloslavskiy wrote:

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

quoted

-	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:

It is not entirely obvious to me what exactly you mean by "this",
assuming that you refer to comparing two pointers via `<`, I agree that
this is not the best idea, I changed it to `off < pend - msg`. If my
assumption is correct, however, we are still talking about C, so I
wonder how this C++ document you linked could bear any relevance:

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.

quoted

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.

Quite honestly, I purposefully refrained from copying that information.
While even the following paragraphs are incomplete by necessity (there
are many more standard C libraries we try to work against, e.g. musl and
newlibc, and it would be insanity to try to cover them all in an
analysis of stdio buffering), the only finding that is relevant to the
patch under discussion is that MSVC's runtime outputs to `stderr` byte
by byte (or more correctly, character by character, I guess), and I did
mention this in the commit message (in the part you quoted).

Thanks,
Dscho

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]