[Libvirt-cim] [PATCH 4 of 4] Add suport for CreateChildResourcePool

[Kaitlin Rupert]

>> +
>> +        free(pool->pool_info.net.ip_end);
>> +        pool->pool_info.net.ip_end = strdup(val);
> 
> 
> If you plan to keep the duplicated strdups, you should free the alloc'ed 
> memory here.

Yuck.. this was a pretty sloppy patch set.  Thanks for catching the 
strdup() bit!

> 
>> +
>> +        return msg;
> 
> This will always return a NULL value. But you probably add this to be 
> set when address validation fails, right?

Right - it's possible that in the future one of these attributes (or a 
new attribute) will be mandatory.


>> +
>> +        s = get_pool_by_name(_BROKER, reference, full_id, &inst);
>> +        if (s.rc == CMPI_RC_OK) {
>> +                cu_statusf(_BROKER, &s,
>> +                           CMPI_RC_ERR_FAILED,
>> +                           "Pool with that name already exists");
>> +                goto out;
>> +        }
>> +
>> +        pool->id = strdup(name);
> 
> You can use name directly, since the create_child_pool_parse_args 
> already allocs memory for the name var. Or, if you decide to keep that 
> way, free memory from the name var after this.

There should a function to cleanup the virt_pool struct.  I'll add that.

> 
>> +
>> +        xml = pool_to_xml(pool);
>> +        if (xml == NULL) {
>> +                cu_statusf(_BROKER, &s,
>> +                           CMPI_RC_ERR_FAILED,
>> +                           "Unable to generate XML for resource pool");
>> +                goto out;
>> +        }
>> +
>> +        CU_DEBUG("Pool XML:\n%s", xml);
>> +
>> +        inst = connect_and_create(xml, reference, full_id, 
>> pool->type, &s);
>> +        if (inst == NULL) {
>> +                cu_statusf(_BROKER, &s,
>> +                           CMPI_RC_ERR_FAILED,
>> +                           "Unable to create resource pool");
>> +                goto out;
>> +        }
>> +
>> +        result = CMGetObjectPath(inst, &s);
>> +        if ((result != NULL) && (s.rc == CMPI_RC_OK)) {
>> +                CMSetNameSpace(result, NAMESPACE(reference));
>> +                CMAddArg(argsout, "Pool", &result, CMPI_ref);
>> +        }
>> +
>> +        /* FIXME:  Trigger indication here */
>> +
>> +        cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
>> + out:
>> +        free(xml);
>> +        free(full_id);
> 
> Missing free(msg), free(pool)
> How about free(inst)?

msg is a const - no need to free it.

Good call on freeing the pool though - there should be a function that 
handles cleaning up the pool struct elements.


-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com