Re: [PATCH v3 1/1] vreportf(): avoid relying on stdio buffering

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > Also please note that we `fflush(stderr)` here to help when running in a
> > Git Bash on Windows: in this case, `stderr` is not actually truly
> > unbuffered, and needs the extra help.
>
> I do not think you have to single out Windows in this case; if we
> are writing directly to file descriptor #2, as long as there have
> been something written by the process to the standard error stream
> before our caller called us, we must flush it to make sure what
> we are going to write out will come after, no matter what platform
> we are on.
>
>     The process may have written to the standard error stream 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.
>
> or something along that line would be sufficient.

> > diff --git a/usage.c b/usage.c
> > index 2fdb20086b..471efb2de9 100644
> > --- a/usage.c
> > +++ b/usage.c
> > @@ -9,14 +9,21 @@
> >  void vreportf(const char *prefix, const char *err, va_list params)
> >  {
> >  	char msg[4096];
> > -	char *p;
> > +	size_t off = strlcpy(msg, prefix, sizeof(msg));
> > +	char *p, *pend = msg + sizeof(msg);
> >
> > -	vsnprintf(msg, sizeof(msg), err, params);
> > -	for (p = msg; *p; p++) {
> > +	p = off < pend - msg ? msg + off : pend - 1;
> > +	if (vsnprintf(p, pend - p, err, params) < 0)
> > +		*p = '\0'; /* vsnprintf() failed, clip at prefix */
> > +
> > +	for (; p != pend - 1 && *p; p++) {
>
> It may make the result much simpler to start with something like:
>
> 	size_t prefix_len = strlen(prefix);
>
> 	if (sizeof(msg) <= prefix_len) {
>  		fprintf(stderr, "BUG!!! too long a prefix '%s'\n", prefix);
> 		abort();
> 	}
> 	memcpy(msg, prefix, prefix_len);
> 	p = msg + prefix_len;
>
> as we agreed that we won't have prefix that will fill or overflow
> msg[] based on your earlier reading of the callers like BUG-fl.

> >  		if (iscntrl(*p) && *p != '\t' && *p != '\n')
> >  			*p = '?';
> >  	}
>
> Thanks.  With this flow it is crystal clear that the prefix is shown
> as-is, while the payload is sanitized.

> > -	fprintf(stderr, "%s%s\n", prefix, msg);
> > +
> > +	*(p++) = '\n'; /* we no longer need a NUL */
> > +	fflush(stderr);
> > +	write_in_full(2, msg, p - msg);
> >  }
> >
> >  static NORETURN void usage_builtin(const char *err, va_list params)
>
>