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/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
commit e74427b0570f4389b77c035dc093312f90b3d1e1 Author: Daniel Veillard <veill...@redhat.com> Date: Tue Nov 10 14:40:22 2009 +0100 Various fixes following a code review part 2 * daemon/libvirtd.c tools/virsh.c: Steve Grubb <sgr...@redhat.com> found a few more issues diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index daf06bc..2fcd9a9 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -359,7 +359,6 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED, void *opaque) { struct qemud_server *server = (struct qemud_server *)opaque; siginfo_t siginfo; - int ret; virMutexLock(&server->lock); @@ -371,8 +370,6 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED, return; } - ret = 0; - switch (siginfo.si_signo) { case SIGHUP: VIR_INFO0(_("Reloading configuration on SIGHUP")); @@ -392,9 +389,6 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED, break; } - if (ret != 0) - server->quitEventThread = 1; - virMutexUnlock(&server->lock); } diff --git a/tools/virsh.c b/tools/virsh.c index f8e6ce4..0d0ebca 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -4627,8 +4627,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) if (format) { virBufferAddLit(&buf, " <target>\n"); - if (format) - virBufferVSprintf(&buf, " <format type='%s'/>\n",format); + virBufferVSprintf(&buf, " <format type='%s'/>\n",format); virBufferAddLit(&buf, " </target>\n"); } virBufferAddLit(&buf, "</volume>\n"); @@ -6835,6 +6834,7 @@ editWriteToTempFile (vshControl *ctl, const char *doc) if (fd == -1) { vshError(ctl, _("mkstemp: failed to create temporary file: %s"), strerror(errno)); + free (ret); return NULL; } @@ -7675,7 +7675,7 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, *name = n; /* try it by UUID */ - if (network==NULL && (flag & VSH_BYUUID) && strlen(n)==VIR_UUID_STRING_BUFLEN-1) { + if ((flag & VSH_BYUUID) && (strlen(n) == VIR_UUID_STRING_BUFLEN-1)) { vshDebug(ctl, 5, "%s: <%s> trying as network UUID\n", cmd->def->name, optname); network = virNetworkLookupByUUIDString(ctl->conn, n); @@ -7715,7 +7715,7 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, *name = n; /* try it by NAME */ - if ((iface == NULL) && (flag & VSH_BYNAME)) { + if ((flag & VSH_BYNAME)) { vshDebug(ctl, 5, "%s: <%s> trying as interface NAME\n", cmd->def->name, optname); iface = virInterfaceLookupByName(ctl->conn, n); @@ -7752,13 +7752,13 @@ vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, *name = n; /* try it by UUID */ - if (pool==NULL && (flag & VSH_BYUUID) && strlen(n)==VIR_UUID_STRING_BUFLEN-1) { + if ((flag & VSH_BYUUID) && (strlen(n) == VIR_UUID_STRING_BUFLEN-1)) { vshDebug(ctl, 5, "%s: <%s> trying as pool UUID\n", cmd->def->name, optname); pool = virStoragePoolLookupByUUIDString(ctl->conn, n); } /* try it by NAME */ - if (pool==NULL && (flag & VSH_BYNAME)) { + if (pool == NULL && (flag & VSH_BYNAME)) { vshDebug(ctl, 5, "%s: <%s> trying as pool NAME\n", cmd->def->name, optname); pool = virStoragePoolLookupByName(ctl->conn, n);
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list