[libvirt] PATCH: Fix NULL checks in openvz driver

[Evgeniy Sokolov]

> On Fri, Sep 05, 2008 at 05:40:16PM +0200, Jim Meyering wrote:
>> "Daniel P. Berrange" <berrange at 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;
>>>
>> ...
>>
>> 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);
> 
It looks very good.