[libvirt] Re: libvirt code review
On Thu, Nov 05, 2009 at 01:12:41PM -0500, Steve Grubb wrote:
Hello,
The following is against 0.7.2-3 from F-13 cvs. I mention each source file at the beginning of a block and have a blank line indicating that we are moving along to the next file. Hope you find this helpful.
-Steve
Okay I fixed the issues on a 0.7.2 branch and then backported the patch back onto git head, except for a few exceptions, everything was still applying !
In src/util/storage_file.c at line 192, there is a test that I presume is for -1. The problem is that its a variable that is an unsigned long is 64 bits on
unsigned long size; size = ((buf[4+4+8] << 24) | (buf[4+4+8+1] << 16) | (buf[4+4+8+2] << 8) | buf[4+4+8+3]); /* QCowHeader.backing_file_size */ [...] if (size + 1 == 0) return BACKING_STORE_INVALID; if (VIR_ALLOC_N(*res, size + 1) < 0) { I think it's just to make 100% sure the size + 1 computation on the following line won't overflow. So current code looks fine to me
some arches. It would probably be better to use a unit32_t for size or change the test. The bottom line is the test will evaluate false on some arches.
unit32_t is nowhere used in libvirt code I would rather not use it, we could use size_t there since the goal is to allocate memory but I'm not 100% sure it's right, basically the test make sure we won't try to allocate more than 4G of memory in one chunk.
In src/datatypes.c at lines 335, 476, and 790, there is a test to see if uuid is NULL. There was a test in the beginning of the function and it returned if this was true, so this "if" statement can be deleted in all 3 cases.
Hum, this is the virGetxxx() functions, they lookup objects given the name or uuid, and hold big TODOs comments at the start: /* TODO search by UUID first as they are better differenciators */ /* TODO check the UUID */ And somehow I think the intend was to allow lookup only by name, with the uuid being unknown, I would think the proper fix is actually to drop the || (uuid == NULL) at the start of the function.
In src/libvirt.c at line 1122, there is an "if" statement that "ands" 0 with the return from STREQ. Should that have been a == ? The test evaluates false as is.
Comparing with others similar message being emitted in other places, I think '0 && ' is just a bogus remain, I'm removing it.
At line 3214, there is a test if uri == NULL. Then within that "if" statement at line 3216 is a test for if uri is still NULL. Should that have been a test for dstURI being NULL?
yup ! That had been fixed in head since though.
In src/remote/remote_driver.c at line 939 is a do loop. By design, it seems the intention was to reloop if waitpid returns -1 and EINTR is set. The implementation doesn't match it. If -1 is returned, it will do the continue, which is to jump to the end of the do loop and it evaluates reap which would be a -1 and it will exit. I suspect instead of continue, it should "goto again" and again would be placed before the call to waitpid. This problem is found again at line 1406
okay, nasty, somehow it's easy to think continue will retry from top of the loop not including the test
In src/xen/sexpr.c at line 443 is a test for token == NULL. Token is guaranteed to not be NULL because of the loop entry test at line 440.
okay
In src/xen/xend_internal.c at line 1367 is a test for connectPort being true. Prior to this, connectPort is guranteed to be valid or it would have jumped to no_memory.
okay as the affectation is done in both if and else parts of the preceeding test.
In src/xen/xm_internal.c at line 579 is a test for dh being valid. It is guaranteed to be valid since a -1 was returned from the function on a failed call to opendir.
okay
At line 2219 is an unusual if statement. Normally you do not see something constructed as (!cpu)<0). That would seem to have meant cpu>=0 which is more straightforward.
if (def->cpumask && !(cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) < 0) goto cleanup; But the function returns a string or NULL in case of error, I try to fight again not fully parenthetized test expressions but I'm loosing that fight unfortunately :-( in any case the < 0 test is bogus since it's a string if ((def->cpumask) && ((cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) == NULL)) goto cleanup seems the right construct to me, as it tests if the call failed.
In src/phyp/phyp_driver.c at line 1008, there is a strdup inside an if statement with nothing catching its returned memory. Two lines later it does the same thing again but puts it in an array. Not sure what the if statement should have been checking, but its definitely a memory leak as is. At line 1040 is an allocation. If that fails, it passes dom->comm to an error reporting function. However, at line 1034, dom was set to NULL and is not changed. The same problem appears again at line 1085. At line 1105 is a test for exit_status < 0. Nothing has changed it since it was initialized.
That code was completely revamped since, none of this applies now, it was pretty bad ! C.f. https://bugzilla.redhat.com/show_bug.cgi?id=529977
In src/openvz/openvz_conf at line 336 is a test for from being NULL. However, it was previously used at line 333 before checking if it was NULL.
yeah, there was another passed pointer which should have got the same check too, cleaning this up.
In src/qemu/qemu_monitor_text.c at line 178, a variable, control, is assigned to a member of the msg structure. However, control goes out of scope before its used. It should be in the function level declarations.
Ah right, nasty !
In src/qemu/qemu_driver.c at line 894 is a test for retries being 0. This will always be true since the loop has no break statements. At line 4484, we leave the function without freeing devname.
okay,
In src/lxc/lxc_conf.c at line 111, 114, and 125, we leave the function without freeing filename.
okay
In src/lxc/lxc_container.c at line 743, we leave the function without freeing ttyPath.
okay
In src/storage/storage_driver.c at lines 83 and 93 we call storageLog/fprintf with the second parameter to %s being NULL. Wasn't an empty string intended?
I used "no error message found" as we failed to gather an error message
In src/storage/storage_backend_disk.c at line 88, we leave the function without freeing devpath.
okay
At line 649 is a test for part_num being NULL. Can it ever be NULL?
No, but we should check for an empty string, done
In src/node_device/node_device_hal.c at line 405 is a test for hal_cap_names being true, its conceivable to get there without it being initialized. For example, line 373 could jump to it. It should probably be initialized to 0.
okay
At line 781 is a test for udi being true. It will alwys be false since once its defined it will never jump to failure. At line 782 is a use of num_devs without it being initialized. If the problem mentioned for udi is fixed by deleting all this code, then the problem goes away.
okay removing that block, I just hope we won't add a failure jump from within the loop
In src/lxc/lxc_controller.c at line 633 is a test for containerPty not being -1. Its conceivable to get here with it notr being initialized perhaps at line 512 for example. It should probably be initialized to a -1.
yup, okay
At line 735 is an if statement "anding" 0 with getuid(). this should probably be a != rather than a &&.
right since the error message clearly indicate we expect this to run as root. Thanks a lot for the review ! Patch for git head attached Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tuesday 10 November 2009 07:00:36 am Daniel Veillard wrote:
At line 2219 is an unusual if statement. Normally you do not see something constructed as (!cpu)<0). That would seem to have meant cpu>=0 which is more straightforward.
if (def->cpumask && !(cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) < 0) goto cleanup;
But the function returns a string or NULL in case of error, I try to fight again not fully parenthetized test expressions but I'm loosing that fight unfortunately :-(
in any case the < 0 test is bogus since it's a string
if ((def->cpumask) && ((cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) == NULL)) goto cleanup
seems the right construct to me, as it tests if the call failed.
This looks a lot better. Thanks for looking into all these findings. I'll try to do another sweep in a few months to see if anything new pops up. -Steve
On Tue, Nov 10, 2009 at 11:07:31AM -0500, Steve Grubb wrote:
On Tuesday 10 November 2009 07:00:36 am Daniel Veillard wrote:
At line 2219 is an unusual if statement. Normally you do not see something constructed as (!cpu)<0). That would seem to have meant cpu>=0 which is more straightforward.
if (def->cpumask && !(cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) < 0) goto cleanup;
But the function returns a string or NULL in case of error, I try to fight again not fully parenthetized test expressions but I'm loosing that fight unfortunately :-(
in any case the < 0 test is bogus since it's a string
if ((def->cpumask) && ((cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) == NULL)) goto cleanup
seems the right construct to me, as it tests if the call failed.
This looks a lot better. Thanks for looking into all these findings. I'll try to do another sweep in a few months to see if anything new pops up.
Okay, thanks, just tell me before, as the code tend to move fast, I should be able to point your to a recent release or sugegst a snapshot ftp://libvirt.org/libvirt/libvirt-git-snapshot.tar.gz they are generated every hour or so :-) thanks again ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Nov 10, 2009 at 01:00:36PM +0100, Daniel Veillard wrote:
commit 680ced4bde478006a2e6445801993e110a3687be Author: Daniel Veillard <veillard@redhat.com> Date: Tue Nov 10 12:56:11 2009 +0100
Various fixes following a code review
* src/libvirt.c src/lxc/lxc_conf.c src/lxc/lxc_container.c src/lxc/lxc_controller.c src/node_device/node_device_hal.c src/openvz/openvz_conf.c src/qemu/qemu_driver.c src/qemu/qemu_monitor_text.c src/remote/remote_driver.c src/storage/storage_backend_disk.c src/storage/storage_driver.c src/util/logging.c src/xen/sexpr.c src/xen/xend_internal.c src/xen/xm_internal.c: Steve Grubb <sgrubb@redhat.com> sent a code review and those are the fixes correcting the problems
diff --git a/src/libvirt.c b/src/libvirt.c index 2c50790..9e80e29 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -1122,7 +1122,7 @@ do_open (const char *name, (res == VIR_DRV_OPEN_DECLINED ? "DECLINED" : (res == VIR_DRV_OPEN_ERROR ? "ERROR" : "unknown status"))); if (res == VIR_DRV_OPEN_ERROR) { - if (0 && STREQ(virStorageDriverTab[i]->name, "remote")) { + if (STREQ(virStorageDriverTab[i]->name, "remote")) { virLibConnWarning (NULL, VIR_WAR_NO_STORAGE, "Is the daemon running ?"); } diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index 74dc367..bc81543 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -31,6 +31,7 @@ #include "nodeinfo.h" #include "virterror_internal.h" #include "conf.h" +#include "memory.h" #include "logging.h"
@@ -111,10 +112,10 @@ int lxcLoadDriverConfig(lxc_driver_t *driver)
/* Avoid error from non-existant or unreadable file. */ if (access (filename, R_OK) == -1) - return 0; + goto done; conf = virConfReadFile(filename, 0); if (!conf) - return 0; + goto done;
p = virConfGetValue(conf, "log_with_libvirtd"); if (p) { @@ -125,6 +126,9 @@ int lxcLoadDriverConfig(lxc_driver_t *driver) }
virConfFree(conf); + +done: + VIR_FREE(filename); return 0;
no_memory: diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 97b7903..c77d099 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -753,6 +753,7 @@ static int lxcContainerChild( void *data ) virReportSystemError(NULL, errno, _("Failed to open tty %s"), ttyPath); + VIR_FREE(ttyPath); return -1; } VIR_FREE(ttyPath); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 0b104a1..3cecdce 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -501,7 +501,7 @@ lxcControllerRun(virDomainDefPtr def, { int rc = -1; int control[2] = { -1, -1}; - int containerPty; + int containerPty = -1; char *containerPtyPath; pid_t container = -1; virDomainFSDefPtr root; @@ -734,7 +734,7 @@ int main(int argc, char *argv[]) goto cleanup; }
- if (getuid() && 0) { + if (getuid() != 0) { fprintf(stderr, "%s: must be run as the 'root' user\n", argv[0]); goto cleanup; } diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index fe8d116..818c7d6 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -364,7 +364,7 @@ static int gather_capabilities(LibHalContext *ctx, const char *udi, { char *bus_name = NULL; virNodeDevCapsDefPtr caps = NULL; - char **hal_cap_names; + char **hal_cap_names = NULL; int rv, i;
if (STREQ(udi, "/org/freedesktop/Hal/devices/computer")) { @@ -778,11 +778,6 @@ static int halDeviceMonitorStartup(int privileged ATTRIBUTE_UNUSED) virNodeDeviceObjListFree(&driverState->devs); if (hal_ctx) (void)libhal_ctx_free(hal_ctx); - if (udi) { - for (i = 0; i < num_devs; i++) - VIR_FREE(udi[i]); - VIR_FREE(udi); - } nodeDeviceUnlock(driverState); VIR_FREE(driverState);
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 6eeece8..a50e4d6 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -329,12 +329,14 @@ openvz_replace(const char* str, const char* to) { const char* offset = NULL; const char* str_start = str; - int to_len = strlen(to); - int from_len = strlen(from); + int to_len; + int from_len; virBuffer buf = VIR_BUFFER_INITIALIZER;
- if(!from) + if ((!from) || (!to)) return NULL; + from_len = strlen(from); + to_len = strlen(to);
while((offset = strstr(str_start, from))) { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f022f89..08a9b42 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -886,9 +886,9 @@ qemudReadLogOutput(virConnectPtr conn, usleep(100*1000); retries--; } - if (retries == 0) - qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("Timed out while reading %s log output"), what); + + qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("Timed out while reading %s log output"), what); return -1; }
@@ -4352,6 +4352,7 @@ static int qemudDomainChangeEjectableMedia(virConnectPtr conn, newdisk->src = NULL; origdisk->type = newdisk->type; } + VIR_FREE(devname);
return ret; } diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 35cd330..ab1fb9a 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -159,6 +159,7 @@ qemuMonitorSendUnix(const virDomainObjPtr vm, size_t cmdlen, int scm_fd) { + char control[CMSG_SPACE(sizeof(int))]; struct msghdr msg; struct iovec iov[1]; ssize_t ret; @@ -172,7 +173,6 @@ qemuMonitorSendUnix(const virDomainObjPtr vm, msg.msg_iovlen = 1;
if (scm_fd != -1) { - char control[CMSG_SPACE(sizeof(int))]; struct cmsghdr *cmsg;
msg.msg_control = control; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c866111..ba111d8 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -937,9 +937,10 @@ doRemoteOpen (virConnectPtr conn, if (priv->pid > 0) { pid_t reap; do { +retry: reap = waitpid(priv->pid, NULL, 0); if (reap == -1 && errno == EINTR) - continue; + goto retry; } while (reap != -1 && reap != priv->pid); } #endif @@ -1400,9 +1401,10 @@ doRemoteClose (virConnectPtr conn, struct private_data *priv) if (priv->pid > 0) { pid_t reap; do { +retry: reap = waitpid(priv->pid, NULL, 0); if (reap == -1 && errno == EINTR) - continue; + goto retry; } while (reap != -1 && reap != priv->pid); } #endif diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index e82959c..72ccd81 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -82,12 +82,10 @@ virStorageBackendDiskMakeDataVol(virConnectPtr conn, * dir every time its run. Should figure out a more efficient * way of doing this... */ - if ((vol->target.path = virStorageBackendStablePath(conn, - pool, - devpath)) == NULL) - return -1; - + vol->target.path = virStorageBackendStablePath(conn, pool, devpath); VIR_FREE(devpath); + if (vol->target.path == NULL) + return -1; }
if (vol->key == NULL) { @@ -646,7 +644,7 @@ virStorageBackendDiskDeleteVol(virConnectPtr conn,
part_num = devname + strlen(srcname);
- if (!part_num) { + if (*part_num == 0) { virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, _("cannot parse partition number from target " "'%s'"), devname); diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 80e4543..e15de28 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -80,7 +80,8 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { backend->startPool(NULL, pool) < 0) { virErrorPtr err = virGetLastError(); storageLog("Failed to autostart storage pool '%s': %s", - pool->def->name, err ? err->message : NULL); + pool->def->name, err ? err->message : + "no error message found"); virStoragePoolObjUnlock(pool); continue; } @@ -90,7 +91,8 @@ storageDriverAutostart(virStorageDriverStatePtr driver) { if (backend->stopPool) backend->stopPool(NULL, pool); storageLog("Failed to autostart storage pool '%s': %s", - pool->def->name, err ? err->message : NULL); + pool->def->name, err ? err->message : + "no error message found"); virStoragePoolObjUnlock(pool); continue; } diff --git a/src/util/logging.c b/src/util/logging.c index 4899de6..757f78c 100644 --- a/src/util/logging.c +++ b/src/util/logging.c @@ -386,7 +386,7 @@ int virLogDefineFilter(const char *match, int priority, }
mdup = strdup(match); - if (dup == NULL) { + if (mdup == NULL) { i = -1; goto cleanup; } @@ -484,6 +484,7 @@ int virLogDefineOutput(virLogOutputFunc f, virLogCloseFunc c, void *data,
virLogLock(); if (VIR_REALLOC_N(virLogOutputs, virLogNbOutputs + 1)) { + VIR_FREE(ndup); goto cleanup; } ret = virLogNbOutputs++; diff --git a/src/xen/sexpr.c b/src/xen/sexpr.c index 81cb49f..085500d 100644 --- a/src/xen/sexpr.c +++ b/src/xen/sexpr.c @@ -440,9 +440,6 @@ sexpr_lookup_key(const struct sexpr *sexpr, const char *node) for (token = strsep(&ptr, "/"); token; token = strsep(&ptr, "/")) { const struct sexpr *i;
- if (token == NULL) - continue; - sexpr = sexpr->u.s.cdr; for (i = sexpr; i->kind != SEXPR_NIL; i = i->u.s.cdr) { if (i->kind != SEXPR_CONS || diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 3c660be..dad0784 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1369,16 +1369,14 @@ xend_parse_sexp_desc_char(virConnectPtr conn, goto no_memory; }
- if (connectPort) { - if (connectHost) { - virBufferVSprintf(buf, - " <source mode='connect' host='%s' service='%s'/>\n", - connectHost, connectPort); - } else { - virBufferVSprintf(buf, - " <source mode='connect' service='%s'/>\n", - connectPort); - } + if (connectHost) { + virBufferVSprintf(buf, + " <source mode='connect' host='%s' service='%s'/>\n", + connectHost, connectPort); + } else { + virBufferVSprintf(buf, + " <source mode='connect' service='%s'/>\n", + connectPort); } if (bindPort) { if (bindHost) { diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 5878ba1..f833ce7 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -576,8 +576,7 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) { virHashRemoveSet(priv->configCache, xenXMConfigReaper, xenXMConfigFree, &args); ret = 0;
- if (dh) - closedir(dh); + closedir(dh);
return (ret); } @@ -2229,8 +2228,9 @@ virConfPtr xenXMDomainConfigFormat(virConnectPtr conn, if (xenXMConfigSetInt(conf, "vcpus", def->vcpus) < 0) goto no_memory;
- if (def->cpumask && - !(cpus = virDomainCpuSetFormat(conn, def->cpumask, def->cpumasklen)) < 0) + if ((def->cpumask != NULL) && + ((cpus = virDomainCpuSetFormat(conn, def->cpumask, + def->cpumasklen)) == NULL)) goto cleanup;
if (cpus &&
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, Nov 10, 2009 at 04:43:02PM +0000, Daniel P. Berrange wrote:
On Tue, Nov 10, 2009 at 01:00:36PM +0100, Daniel Veillard wrote:
commit 680ced4bde478006a2e6445801993e110a3687be Author: Daniel Veillard <veillard@redhat.com> Date: Tue Nov 10 12:56:11 2009 +0100
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 35cd330..ab1fb9a 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -159,6 +159,7 @@ qemuMonitorSendUnix(const virDomainObjPtr vm, size_t cmdlen, int scm_fd) { + char control[CMSG_SPACE(sizeof(int))]; struct msghdr msg; struct iovec iov[1]; ssize_t ret; @@ -172,7 +173,6 @@ qemuMonitorSendUnix(const virDomainObjPtr vm, msg.msg_iovlen = 1;
if (scm_fd != -1) { - char control[CMSG_SPACE(sizeof(int))]; struct cmsghdr *cmsg;
msg.msg_control = control; [...]
ACK
Okay, both pushed except for the diff above which has disapeard in the refactoring. thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Nov 10, 2009 at 01:00:36PM +0100, Daniel Veillard wrote:
On Thu, Nov 05, 2009 at 01:12:41PM -0500, Steve Grubb wrote:
In src/qemu/qemu_monitor_text.c at line 178, a variable, control, is assigned to a member of the msg structure. However, control goes out of scope before its used. It should be in the function level declarations.
Ah right, nasty ! [...] diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 35cd330..ab1fb9a 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -159,6 +159,7 @@ qemuMonitorSendUnix(const virDomainObjPtr vm, size_t cmdlen, int scm_fd) { + char control[CMSG_SPACE(sizeof(int))]; struct msghdr msg; struct iovec iov[1]; ssize_t ret; @@ -172,7 +173,6 @@ qemuMonitorSendUnix(const virDomainObjPtr vm, msg.msg_iovlen = 1;
if (scm_fd != -1) { - char control[CMSG_SPACE(sizeof(int))]; struct cmsghdr *cmsg;
msg.msg_control = control;
Actually that patch disapears after Dan last commits, as the function is refactored, uses qemuMonitorIOWriteWithFD() where the issue is fixed. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Steve Grubb