[Libvirt-cim] [PATCH 2 of 2] Adding ElementSettingData association

[Dan Smith]

KR> +        *dom_name = strdup(strchr(id, ':') + 1);

This is a crash waiting to happen.  If strchr() returns NULL because
the ':' isn't found, then it'll try to strdup(NULL + 1), which will
just trample through memory until it hits a '\0' or runs into a
segfault.

KR> +static CMPIStatus vssd_to_sd(const CMPIObjectPath *ref,
KR> +                             struct std_assoc_info *info,
KR> +                             struct inst_list *list)
KR> +{
KR> +        CMPIStatus s;
KR> +        CMPIInstance *inst;
KR> +        virConnectPtr conn = NULL;
KR> +        virDomainPtr dom = NULL;
KR> +        char *host = NULL;
KR> +
KR> +        ASSOC_MATCH(info->provider_name, CLASSNAME(ref));
KR> +
KR> +        s = get_dom_name_from_ref(ref, &host);
KR> +        if (s.rc != CMPI_RC_OK) 
KR> +                goto out;
KR> +
KR> +        conn = lv_connect(_BROKER, &s);
KR> +        if (conn == NULL)
KR> +                goto out;
KR> +
KR> +        dom = virDomainLookupByName(conn, host);
KR> +        if (dom == NULL) {
KR> +                cu_statusf(_BROKER, &s,
KR> +                           CMPI_RC_ERR_FAILED,
KR> +                           "No such system `%s'", host);
KR> +                goto out;
KR> +        }
KR> +
KR> +        inst = get_vssd_instance(dom, _BROKER, ref);
KR> +        if (inst == NULL) {
KR> +                cu_statusf(_BROKER, &s,
KR> +                           CMPI_RC_ERR_FAILED,
KR> +                           "Error getting VSSD for `%s'", host);
KR> +                goto out;
KR> +        }
KR> +
KR> +        inst_list_add(list, inst);
KR> +
KR> + out:
KR> +        virDomainFree(dom);
KR> +        virConnectClose(conn);
KR> +

You leak "host" here.

KR> +static CMPIInstance *make_ref(const CMPIObjectPath *ref,
KR> +                              const CMPIInstance *inst,
KR> +                              struct std_assoc_info *info,
KR> +                              struct std_assoc *assoc)
KR> +{
KR> +        CMPIInstance *refinst = NULL;
KR> +        char *base;
KR> +
KR> +        base = class_base_name(assoc->assoc_class);
KR> +        if (base == NULL)
KR> +                goto out;
KR> +
KR> +        refinst = get_typed_instance(_BROKER,
KR> +                                     base,
KR> +                                     NAMESPACE(ref));
KR> +
KR> +        if (refinst != NULL) {
KR> +                CMPIObjectPath *instop;
KR> +
KR> +                instop = CMGetObjectPath(inst, NULL);
KR> +
KR> +                set_reference(assoc, refinst, ref, instop);
KR> +
KR> +                /* Set additional properties with values
KR> +                 * defined in the "Virtual System Profile."
KR> +                 */
KR> +                set_reference(assoc, refinst, ref, instop);

Are you making the same set_reference() call twice in a row?

KR> +                unsigned int lValue = 1;

Please don't define new variables in the middle of a block.

KR> +                CMSetProperty(refinst, "IsDefault",
KR> +                        (CMPIValue *)&lValue, CMPI_uint16);

"lValue" is an unsigned int, which is not a uint16.  Define lValue as
a uint16_t.

-- 
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/20071114/e5e0ed3b/attachment.sig>