[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



"Daniel P. Berrange" <berrange redhat com> 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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]