[Libvirt-cim] [PATCH 2 of 3] Make alloc_cap_instances() available for external use by EC

[Dan Smith]

HE> +        CMPIStatus s = {CMPI_RC_OK, NULL};
HE> +        CMPIInstance *alloc_cap_inst;
HE> +        virConnectPtr conn = NULL;
HE> +        struct inst_list device_pool_list;
HE> +        const char *inst_id;
HE>          int i;
HE> -        virConnectPtr conn = NULL;
HE> -        CMPIInstance *alloc_cap_inst;
HE> -        struct inst_list alloc_cap_list;
HE> -        struct inst_list device_pool_list;
HE> -        CMPIStatus s = {CMPI_RC_OK, NULL};
HE> -        const char *inst_id;

Reordering this block for no reason makes it really difficult to tell
what is changed.  Can we please keep this kind of thing separate from
the functional change to the code?

HE> +        inst_list_init(list);

In other places, we initialize the list before we pass it into a
function.  I think it makes more sense that way, as the caller may
want the results of this function appended to the list it passes in.

HE> +static CMPIStatus return_alloc_cap_instances(const CMPIBroker *broker,
HE> +                                             const CMPIObjectPath *ref,
HE> +                                             const CMPIResult *results,
HE> +                                             bool names_only,
HE> +                                             const char **properties,
HE> +                                             const char *id)
HE> +{
HE> +        CMPIStatus s = {CMPI_RC_OK, NULL};
HE> +        struct inst_list *list = NULL;
HE> +
HE> +        s = enum_alloc_cap_instances(broker,
HE> +                                     ref,
HE> +                                     properties,
HE> +                                     id,
HE> +                                     list);
HE> +        if (s.rc != CMPI_RC_OK)
HE> +                goto out;
HE> +        
HE> +        if (names_only)
HE> +                cu_return_instance_names(results, list);
HE> +        else
HE> +                cu_return_instances(results, list);

This doesn't work, does it?  You pass a NULL list pointer in to the
function, it allocates a list, but has no way to return it back.
You're passing just a single pointer by value here.  If you were
modifying what the pointer pointed to, then you could get the result,
but otherwise the list that the enum_alloc_cap_instances() function
generates isn't visible to this caller.

You should do the following:

  struct inst_list list;

  inst_list_init(&list);

  s = enum_alloc_cap_instances(broker,
                               ref,
                               properties,
                               id,
                               &list);

Which addresses this issue as well as the previous one.

Thanks!

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms at us.ibm.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20080111/05d2adf4/attachment.sig>