vreportf: Fix interleaving issues

Previously, `vreportf()` wasn't atomic enough. Depending on `fprintf()`
implementation and `stderr` type (console or file), that could lead to
interleaved and sometimes hard to read outputs.

This fixes user visible character-interleaved output on Windows.
This also fixes t5516 on Windows.

Example
-------
This output was produced by t5516 on Windows, where both `git fetch`
and child `git upload-pack` call `die()` at the same time:

	fatal: git uploadfata-lp: raemcokte :error:  upload-pnot our arcef k6: n4ot our ea4cr1e3f 36d45ea94fca1398e86a771eda009872d63adb28598f6a9
	8e86a771eda009872d6ab2886

Idea behind this patch
----------------------
Pre-format everything to memory and use low-level `xwrite()` to send
string to `stderr` in a more atomic way. I'm not sure if `xwrite()`
has guarantees to be atomic, but this is definitely better then
`fprintf()` and has shown to work fine.

Message trimming
----------------
Initially I wrote code to also support messages of any length. However,
during patch discussion both Johannes Schindelin and Jeff King voted
against extra complexity added by trying to dodge failing `malloc`
and still report something to user. It is not expected that a
reasonable error message can exceed 4096 bytes and still be better then
a trimmed message.

Pitfalls
--------
1) `vsnprintf()` returns desired size instead of trimmed size when
   full output is too long to fit. In that case, the returned value
   must not be used for indexing.
2) If `vsnprintf()` was trimmed, `msg` could contain uninitialized
   bytes at the end if last character that didn't fit was a multibyte
   character. In this case, fitting '\n' as last character of `msg`
   could result in sending corrupt multibyte data.
3) `strlcpy()`, which could be used instead of `memcpy()`, also returns
   desired size instead of trimmed size. This again makes an easy
   pitfall in the next line with `vsnprintf()`.

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
---------------------------------------
a) libc - large outputs can be block-interleaved
   fprintf() enables temporary stream buffering.
   Code references:
       buffered_vfprintf()
b) 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()
c) 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]

History of interleaving in `vreportf()`
---------------------------------------
d048a96 (2007-11-09) - `char msg[256]` used to avoid interleaving,
                        but it still uses `fprintf()` and issues remain
389d176 (2009-03-25) - Buffer size increased to 1024 to avoid truncation
625a860 (2009-11-22) - Buffer size increased to 4096 to avoid truncation
f4c3edc (2015-08-11) - Buffer removed to avoid truncation
b5a9e43 (2017-01-11) - Reverts f4c3edc to be able to replace control
                        chars before sending to `stderr`

Non `vreportf()` commits worthy of notice
-----------------------------------------
9ac13ec (2006-10-11) - Another attempt to solve interleaving.
                        This is seemingly related to d048a96.
137a0d0 (2007-11-19) - Addresses out-of-order for `display()`
34df8ab (2009-03-10) - Switches `xwrite()` to `fprintf()` in
                        `recv_sideband()` to support UTF-8 emulation
eac14f8 (2012-01-14) - Removes the need for `fprintf()` for UTF-8
                        emulation, so it's safe to use `xwrite()` again
5e5be9e (2016-06-28) - `recv_sideband()` uses `xwrite()` again

Co-authored-by: Johannes Schindelin <[email protected]>
Signed-off-by: Alexandr Miloslavskiy <[email protected]>

Author: SyntevoAlex

usage.c

@@ -6,17 +6,40 @@
 #include "git-compat-util.h"
 #include "cache.h"
 
+static void replace_control_chars(char *str, size_t size)
+{
+	size_t i;
+
+	for (i = 0; i < size; i++) {
+		if (iscntrl(str[i]) && str[i] != '\t' && str[i] != '\n')
+			str[i] = '?';
+	}
+}
+
+/* Atomically report (prefix + vsnprintf(err, params) + '\n') to stderr. */
 void vreportf(const char *prefix, const char *err, va_list params)
 {
 	char msg[4096];
-	char *p;
+	int ret;
+	size_t prefix_size, total_size;
 
-	vsnprintf(msg, sizeof(msg), err, params);
-	for (p = msg; *p; p++) {
-		if (iscntrl(*p) && *p != '\t' && *p != '\n')
-			*p = '?';
-	}
-	fprintf(stderr, "%s%s\n", prefix, msg);
+	prefix_size = strlen(prefix);
+	if (prefix_size >= sizeof(msg))
+		BUG("vreportf: prefix is too long");
+
+	memcpy(msg, prefix, prefix_size);
+
+	ret = vsnprintf(msg + prefix_size, sizeof(msg) - prefix_size, err, params);
+	if (ret < 0)
+		BUG("your vsnprintf is broken (returned %d)", ret);
+
+	total_size = strlen(msg);   /* vsnprintf() returns _desired_ size */
+	msg[total_size++] = '\n';   /* it's ok to overwrite terminating NULL */
+
+	replace_control_chars(msg, total_size);
+
+	fflush(stderr);             /* flush FILE* before writing lowlevel fd */
+	xwrite(2, msg, total_size); /* writing directly to fd is most atomic */
 }
 
 static NORETURN void usage_builtin(const char *err, va_list params)