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

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote:
> "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;
> >
> ...
> 
> 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);




-- 
|: 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 :|

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list