On Tue, 14 Jun 2016 at 20:11:12, Junio C Hamano wrote: > Nicolas Pitre <nico@xxxxxxxxxxx> writes: > > > It is not buffered as it writes to stderr. And some C libs do separate > > calls to write() for every string format specifier. So "%s%s%c" may end > > up calling write() 3 times depending on the implementation. The example > > I gave in commit ed1902ef5c is real and I even observed it with strace > > back then. > > I think you meant 9ac13ec9 (atomic write for sideband remote > messages, 2006-10-11). > > IIRC, back then we did't use to make as much use of strbuf API as we > do today; if we were doing that commit today, we would be doing > strbuf, I would suspect. Thanks for looking up the right commit. I think I might see what is going on; however, the situation is a bit different to the situation we had at the time of writing 9ac13ec9 (atomic write for sideband remote messages, 2006-10-11). Before that, we called write() manually a couple of times. Now, we use fprintf() which makes stronger guarantees about thread safety. However, if I understand correctly, there is still one issue: It seems like in some places, we also directly write() to file descriptor 2. So the following might happen: An fprintf() implementation that calls write() once per format specifier is used. The fprintf() function is called once from one thread, then interrupted between two write() syscalls. In the meantime, output is written directly to file descriptor 2 using write(). One possible solution is using strbuf and constructing the message as we did before. However, that still relies on fprintf() only calling write() once per format specifier. While that is probably true for all existing implementations, I don't think it is guaranteed by some standard. Shouldn't we always use the stderr stream when printing error messages instead, especially when we care about thread safety? Regards, Lukas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html