[Libvirt-cim] [PATCH 1 of 2] Add Modified to the list of available indications in ComputerSystemIndication

Jay Gagnon grendel at linux.vnet.ibm.com
Tue Jan 22 15:43:34 UTC 2008


Dan Smith wrote:
> JG> +static void cleanup_domain_list(virDomainPtr *list, int size)
> JG> +{
> JG> +        int i;
> JG> +
> JG> +        for (i = 0; i < size; i++) {
> JG> +                virDomainFree(list[i]);
> JG> +        }
> JG> +}
>
> This should be in misc_util with the other domain_list functions.
>   
Okay.
> JG> @@ -221,15 +409,34 @@ static CMPI_THREAD_RETURN lifecycle_thre
> JG>                                            conn,
> JG>                                            virDomainGetName(prev_list[i]),
> JG>                                            CS_DELETED);
> JG> -                        virDomainFree(prev_list[i]);
> JG>                  }
>
> Was this intentional?  Cleaning up the list as we go prevents us from
> having to go back through it again.  I'm not sure if I see the items
> used later, so it seems like we could eliminate the second trip
> through with cleanup_domain_list().
>   
I think this is a victim of the two patches working on the same code,
and/or this change probably belongs in the second patch.  When I do the
more complete merge, all three loops work on the lists of dom_xml
structs, so the only time the virDomaintPtr lists get used is when they
are converted to dom_xml lists.
> JG> +        /* Should I free args as well here, since it's malloced in activate? */
>
> Yes :)
>   
Yea I was pretty sure on that.  I mean it definitely needed to be freed,
I just wasn't positive that this was the right spot.

-- 

-Jay




More information about the Libvirt-cim mailing list