Re: [libvirt] PATCH: Fix NULL checks in openvz driver

On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote: > "Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: >> Jim pointed out some places where the openvz driver could deference some >> NULs, so this patch fixes them > ... >> Index: src/openvz_driver.c >> =================================================================== >> RCS file: /data/cvs/libvirt/src/openvz_driver.c,v >> retrieving revision 1.46 >> diff -u -p -r1.46 openvz_driver.c >> --- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46 >> +++ src/openvz_driver.c 5 Sep 2008 15:11:34 -0000 >> @@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma >> static int openvzDomainShutdown(virDomainPtr dom) { >> struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; >> virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); >> - const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL}; >> + const char *prog[] = {VZCTL, "--quiet", "stop", NULL /* name */, NULL}; > ... >> + prog[3] = vm->def->name; >> if (virRun(dom->conn, prog, NULL) < 0) >> return -1; >> > ... > > All looks correct. > However, I'm a little nervous about hard-coding those '[3]'s. > What if someone inserts a new --foo option somewhere before > the NULL place-holder, or otherwise rearranges the options? > Then we'll have to remember to update that "3" below to e.g., "4". > Easy to miss that... I just realized there is a much simpler way todo this... Daniel Index: src/openvz_driver.c =================================================================== RCS file: /data/cvs/libvirt/src/openvz_driver.c,v retrieving revision 1.46 diff -u -p -r1.46 openvz_driver.c --- src/openvz_driver.c 5 Sep 2008 15:00:14 -0000 1.46 +++ src/openvz_driver.c 8 Sep 2008 09:44:53 -0000 @@ -276,7 +276,7 @@ static char *openvzDomainDumpXML(virDoma static int openvzDomainShutdown(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "stop", vm->def->name, NULL}; + const char *prog[] = {VZCTL, "--quiet", "stop", vm ? vm->def->name : NULL, NULL}; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -303,7 +303,7 @@ static int openvzDomainReboot(virDomainP unsigned int flags ATTRIBUTE_UNUSED) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); - const char *prog[] = {VZCTL, "--quiet", "restart", vm->def->name, NULL}; + const char *prog[] = {VZCTL, "--quiet", "restart", vm ? vm->def->name : NULL, NULL}; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -581,7 +581,7 @@ openvzDomainCreate(virDomainPtr dom) { struct openvz_driver *driver = (struct openvz_driver *)dom->conn->privateData; virDomainObjPtr vm = virDomainFindByName(driver->domains, dom->name); - const char *prog[] = {VZCTL, "--quiet", "start", vm->def->name, NULL }; + const char *prog[] = {VZCTL, "--quiet", "start", vm ? vm->def->name : NULL, NULL }; if (!vm) { openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, @@ -614,7 +614,7 @@ openvzDomainUndefine(virDomainPtr dom) virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "destroy", vm->def->name, NULL }; + const char *prog[] = { VZCTL, "--quiet", "destroy", vm ? vm->def->name : NULL, NULL }; if (!vm) { openvzError(conn, VIR_ERR_INVALID_DOMAIN, _("no domain with matching uuid")); @@ -643,7 +643,7 @@ openvzDomainSetAutostart(virDomainPtr do virConnectPtr conn= dom->conn; struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); - const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name, + const char *prog[] = { VZCTL, "--quiet", "set", vm ? vm->def->name : NULL, "--onboot", autostart ? "yes" : "no", "--save", NULL }; @@ -704,16 +704,9 @@ static int openvzDomainSetVcpus(virDomai struct openvz_driver *driver = (struct openvz_driver *) conn->privateData; virDomainObjPtr vm = virDomainFindByUUID(driver->domains, dom->uuid); char str_vcpus[32]; - const char *prog[] = { VZCTL, "--quiet", "set", vm->def->name, + const char *prog[] = { VZCTL, "--quiet", "set", vm ? vm->def->name : NULL, "--cpus", str_vcpus, "--save", NULL }; - snprintf(str_vcpus, 31, "%d", nvcpus); - str_vcpus[31] = '\0'; - if (nvcpus <= 0) { - openvzError(conn, VIR_ERR_INTERNAL_ERROR, - _("VCPUs should be >= 1")); - return -1; - } if (!vm) { openvzError(conn, VIR_ERR_INVALID_DOMAIN, @@ -721,6 +714,15 @@ static int openvzDomainSetVcpus(virDomai return -1; } + if (nvcpus <= 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("VCPUs should be >= 1")); + return -1; + } + + snprintf(str_vcpus, 31, "%d", nvcpus); + str_vcpus[31] = '\0'; + if (virRun(conn, prog, NULL) < 0) { openvzError(conn, VIR_ERR_INTERNAL_ERROR, _("Could not exec %s"), VZCTL);