[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] Re: [Xen-changelog] use format printf style to write to tracefd instead of using write syscall.



On Tue, Jan 24, 2006 at 08:14:40PM +0200, Muli Ben-Yehuda wrote:
> > +   /* try to use a static buffer */
> 
> sbuf isn't static here...

static as contrary to dynamicly allocated, not as a static C variable

do you prefer s/static/stack/ ?

> vsnprintf can return a negative error value on output failure, in
> which case calling write is not a good idea.

Did you really saw a *vSnprintf* fail ever with a negative value ?
I cannot find any reason that it will have an output error, which is the
only reason listed in the manual, since the output is a string in memory.

I think the return value of -1 is only related to:
fprintf, vfprintf, printf (redirection of output), vprintf

If that's not true, I think that there's lots of other place that can
have this problem.

> Also, a return value of
> 1024 (sizeof(sbuf) instead?) or more means the output was truncated,
> not sure if we want to do anything about it.

If it was truncated, that because we force it to write no more than 1024
chars. the stack buffer is just there to avoid dynamic buffer
allocation in the majority of case.

just writing few chars without doing a malloc(N) is better I think,
but speed is not an issue here.

> Also also, do we care if write() failed or did a short write?

I don't think so, but I don't see why we shouldn't try to write
everything if we can, even at the price of a dynamic allocation .. if
dynamic memory allocation fail, we could just try to write what we can
and don't really care.

> trace("%s", "\n***\n")? it's slightly slower but more future proof.

well it could not be that slower anyway actually:

the algo can just strlen+strcpy the first argument for the output
instead of going through the format string char by char for an escape.
for a short string that doesn't really matters though.

I'm happy either way, but patch welcome, although that's just nitpicking
now ;)

-- 
Vincent Hanquez

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.