Skip to content

Commit a57c1fd

Browse files
committed
vreportf(): avoid relying on stdio buffering
The MSVC runtime behavior differs from glibc's with respect to `fprintf(stderr, ...)` in that the former writes out the message character by character. In t5516, this leads to a funny problem where a `git fetch` process as well as the `git upload-pack` process spawned by it _both_ call `die()` at the same time. The output can look like this: fatal: git uploadfata-lp: raemcokte :error: upload-pnot our arcef k6: n4ot our ea4cr1e3f 36d45ea94fca1398e86a771eda009872d63adb28598f6a9 8e86a771eda009872d6ab2886 Let's avoid this predicament altogether by rendering the entire message, including the prefix and the trailing newline, into the buffer we already have (and which is still fixed size) and then write it out via `write_in_full()`. We still clip the message to at most 4095 characters. The history of `vreportf()` with regard to this issue includes the following commits: d048a96 (2007-11-09) - 'char msg[256]' is introduced to avoid interleaving 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 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 Note that we print nothing if the `vsnprintf()` call failed to render the error message; There is little we can do in that case, and it should not happen anyway. The process may have written to `stderr` and there may be something left in the buffer kept in the stdio layer. Call `fflush(stderr)` before writing the message we prepare in this function. Helped-by: Jeff King <[email protected]> Helped-by: Alexandr Miloslavskiy <[email protected]> Helped-by: SZEDER Gábor <[email protected]> Helped-by: Junio C Hamano <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
1 parent 612edef commit a57c1fd

File tree

1 file changed

+16
-4
lines changed

1 file changed

+16
-4
lines changed

usage.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,26 @@
99
void vreportf(const char *prefix, const char *err, va_list params)
1010
{
1111
char msg[4096];
12-
char *p;
12+
char *p, *pend = msg + sizeof(msg);
13+
size_t prefix_len = strlen(prefix);
1314

14-
vsnprintf(msg, sizeof(msg), err, params);
15-
for (p = msg; *p; p++) {
15+
if (sizeof(msg) <= prefix_len) {
16+
fprintf(stderr, "BUG!!! too long a prefix '%s'\n", prefix);
17+
abort();
18+
}
19+
memcpy(msg, prefix, prefix_len);
20+
p = msg + prefix_len;
21+
if (vsnprintf(p, pend - p, err, params) < 0)
22+
*p = '\0'; /* vsnprintf() failed, clip at prefix */
23+
24+
for (; p != pend - 1 && *p; p++) {
1625
if (iscntrl(*p) && *p != '\t' && *p != '\n')
1726
*p = '?';
1827
}
19-
fprintf(stderr, "%s%s\n", prefix, msg);
28+
29+
*(p++) = '\n'; /* we no longer need a NUL */
30+
fflush(stderr);
31+
write_in_full(2, msg, p - msg);
2032
}
2133

2234
static NORETURN void usage_builtin(const char *err, va_list params)

0 commit comments

Comments
 (0)