[Libvirt-cim] [PATCH 4 of 4] Add suport for CreateChildResourcePool
Kaitlin Rupert
kaitlin at linux.vnet.ibm.com
Wed Mar 25 21:53:49 UTC 2009
>> +
>> + 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
More information about the Libvirt-cim
mailing list