"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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; > > @@ -303,7 +304,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", NULL /* name */, NULL}; > > if (!vm) { > openvzError(dom->conn, VIR_ERR_INVALID_DOMAIN, > @@ -317,6 +318,7 @@ static int openvzDomainReboot(virDomainP > return -1; > } > > + prog[3] = vm->def->name; ... > @@ -643,7 +647,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", NULL /* name */, > "--onboot", autostart ? "yes" : "no", > "--save", NULL }; > > @@ -652,6 +656,7 @@ openvzDomainSetAutostart(virDomainPtr do > return -1; > } > > + prog[3] = vm->def->name; ... 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... What do you think about putting a marker string like "NAME" in place of your new NULL, and then iterate through the array of strings searching for the marker; once found, put vm->def->name in that slot; else fail. There are enough uses (6) that it might be worthwhile. -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list