Thread (23 messages) 23 messages, 5 authors, 2019-11-01

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

From: Jeff King <hidden>
Date: 2019-10-29 20:09:38

On Tue, Oct 29, 2019 at 08:57:33PM +0100, Johannes Schindelin wrote:
quoted
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:
I think the issue is not the comparison, but rather that forming the
pointer "msg + off" is undefined, since its point outside of any object
(including the "one past" rule). And this is illegal in both C and C++,
though of course it works fine in practice most of the time.

Doing "off < pend - msg" is legal. Or note that "pend" is just
"msg + sizeof(msg)", subtract out the common term and end up with
"off < sizeof(msg)". :)
quoted
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).
One other case is impacted, which is:
quoted
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.
We'll now output the stderr message closer to its time-of-printing,
which is a good thing (it may still be in a weird place with respect to
a buffered stdout, but getting the error message out ASAP is the best we
can do there).

I agree that the rest of the research is not especially relevant to the
code change (though I'm glad it is available in the list archive).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help