On Wed, Mar 30, 2011 at 13:46:12 -0600, Eric Blake wrote:
On 03/30/2011 06:17 AM, Jiri Denemark wrote:
>
> + if (virAsprintf(&debug, ": %d: debug : ", vm->pid) < 0) {
That's rather hard-coded to our current output style; should we add
comments here and to the place that outputs those lines to be careful to
keep the two places in sync?
Yeah, it makes sense to add these comments.
> - got += ret;
> + got += bytes;
> buf[got] = '\0';
> +
> + /* Filter out debug messages from intermediate libvirt process */
> + while ((eol = strchr(filter_next, '\n'))) {
> + char *p = strstr(filter_next, debug);
Suppose you have five normal lines before the first debug line. Is it
any more efficient to use use getline() to read a line at a time
Heh, the one which needs FILE *? No, thanks :-)
Or even if you don't use getline(), can you temporarily set eol
to NUL
Sure, that actually even results in a bit nicer code.
> + if (p && p < eol) {
> + memmove(filter_next, eol + 1, got - (eol - buf));
Conversely, supposed you have five debug lines followed by five normal
lines. Doing the memmove() on every debug line encountered results in
moving 25 lines around, whereas if you rewrite the loop to only copy one
line at a time and only if it passed the filter, then you only have to
move 5 lines around.
And all this after we went through fork() and going to call exec() soon, I
don't think such optimization would make any difference. If this code had been
run on any libvirt API, it would have make sense to optimize the loop, but now
I don't think it's worth it. It looks like a premature optimization to me :-)
Jirka