I will partially revert commit 44d0532

[Dmitry V. Levin]

On Tue, Jan 24, 2012 at 10:10:05AM +0100, Denys Vlasenko wrote:
> The commit was:
> 
> "strace.c (verror_msg): Rewrite without use of heap memory allocation"
> 
> it replaced "malloc + one fprintf + free" with "three fprintf's".
> 
> I guess it's my fault that I did not add a comment which explains
> that one fprintf was used on purpose: we want to (attempt to)
> send entire message as one write() call. Since stderr is unbuffered,
> separate fprintf's to it always result in separate writes,
> they are not coalesced. If we aren't the only program which
> writes to this particular stderr, this may result in interleaved
> messages.
> 
> Since this function is not performance critical, I guess
> it's ok to make it less efficient.
> 
> I propose the following version:
> 
> 
> static void verror_msg(int err_no, const char *fmt, va_list p)
> {
>         char *msg;
> 
>         fflush(NULL);
> 
>         /* We want to print entire message with single fprintf to ensure
>          * message integrity if stderr is shared with other programs.
>          * Thus we use vasprintf + single fprintf.
>          */

OK with me, but

>         msg = NULL;
>         vasprintf(&msg, fmt, p);
>         if (msg) {

I'd rather replace these three lines with

	if (vasprintf(&msg, fmt, p) >= 0) {

because the value of "msg" is undefined in case of an error, and various
implementations behave quite differently in that case, including a chance
to leave an invalid pointer there.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20120124/209ffcb9/attachment.bin>