[libvirt] Re: libvirt code review
On Thu, Nov 05, 2009 at 03:27:16PM -0500, Steve Grubb wrote:
Hello,
I missed code in a couple other directories. This is much shorter.
-Steve
In daemon/libvirtd.c at line 371 the "if" statement will never be true. ret is
set to 0 at line 350 and never changed.
probably a leftover from previous refactoring, server->shutdown = 1
is directly set in the cases that matters, removed ret altogether.
In the current code head, server->shutdown has been renamed
server->quitEventThread, so patch changed accordingly.
At line 1535 seems to want to loop on reads that return -1 and either
EINTR or
EAGAIN. The implementation does not. Rather than calling return at line 1538,
it should likely have a "goto again" with the again label placed just before
the read call.
Actually in that case I think the current code is fine, the function
should exit 0 on EAGAIN, it's documented, I assume the read will be
retried by the caller, otherwise we would have used saferead() there an
internal routine taking directly care of interrupted calls.
At line 1783 is a call to write that likely wants a treatment similar
to the
read above.
Same thing, I guess it's intended behaviour :-)
In tools/virsh.c at line 4616 is a test that format is true. It was
checked at
line 4614 and nothing changed it.
Ah right, harmless but fixed :-)
At line 6810 memory is allocated. At the error exit at line 6824 it
has not
been freed.
okay
At line 7664 is a test for network==NULL. This will always be true.
At line 7704 is a test for iface == NULL. This will always be true.
At line 7741 is a test for pool==NULL. This will always be true.
Okay, fixed, allows the test to fit in 80 columns too,
Patch for git head attached.
thanks a lot !
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
Attachments:
- review2.patch (text/plain — 3.4 KB)