[Libvirt-cim] [PATCH] This fixes a potential crash if _get_domain() is unable to properly parse the dom XML

[Kaitlin Rupert]

Dan Smith wrote:
> KR>          ret = _get_dominfo(xml, *dominfo);
> KR> +        if (!ret) {
> KR> +                free(*dominfo);
> KR> +                *dominfo = NULL;
> KR> +                return 0;
> KR> +        }
> KR> +
> KR>          free(xml);
>
> You've just introduced a memory leak here, since xml won't get
> free()'d.  Just remove the "return 0" and let it fall through and it
> should be fine.
>   
Yep, good call.  Although, if we fall through here, we run into problems 
with the (*dominfo)->dev_mem_ct = get_mem_devices(dom, 
&(*dominfo)->dev_mem); related pieces since dominfo is NULL.  Instead, I 
think it should be:

         ret = _get_dominfo(xml, *dominfo);
         free(xml);
         if (!ret) {
                free(*dominfo);
                *dominfo = NULL;
                return 0;
         }

>
> KR> -        if (!get_dominfo(dom, &dominfo))
> KR> +        ret = get_dominfo(dom, &dominfo);
> KR> +        if (!ret)
> KR>                  goto out;
>
> Maybe my eyes are tired at the end of the day, but does this fix
> anything or is it just a style change?
>   
We need to capture ret from get_domain().  ret is initialized to 1.  If 
we don't capture the ret from get_domain(), then we return 1 to 
get_vssd_instance().  This causes the if statement to pass, and we 
return an empty instance.  If get_domain() fails, we need to return NULL.

-- 
Kaitlin Rupert
IBM Linux Technology Center
karupert at us.ibm.com