[PATCH 00 of 18] Updates for libcmpiutil API change

Re: [Libvirt-cim] [PATCH 06 of 18]...

Best practices for patchsets

Dan Smith

Tuesday, 27 November 2007 Tue, 27 Nov '07

1:28 p.m.

This refactors all of the necessary functions in libvirt-cim for the new API. Things only got cleaner, I think. I also found a substantial number of memory leaks in the process, which are fundamentally fixed by the new API which I think is a validation of it as "the right thing to do". All required changes should have been easily spotted by the compiler, but some testing would be good. Tip: To find the leaks, note every time we convert a variable from char * to const char * and don't remove a subsequent free() call :)

Show replies by date

Dan Smith

Tuesday, 27 November Tue, 27 Nov

1:28 p.m.

New subject: [PATCH 01 of 18] Fixes to misc_util for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196179380 28800 # Node ID dce4d8a891849228a59d3f90b4b2d30dc098fc3a # Parent dca4cb9c22da97bfef6c2771bf082efbd6d32bc4 Fixes to misc_util for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r dca4cb9c22da -r dce4d8a89184 libxkutil/misc_util.c --- a/libxkutil/misc_util.c Tue Nov 27 10:34:13 2007 +0100 +++ b/libxkutil/misc_util.c Tue Nov 27 08:03:00 2007 -0800 @@ -85,15 +85,18 @@ void free_domain_list(virDomainPtr *list virDomainFree(list[i]); } -char *get_key_from_ref_arg(const CMPIArgs *args, char *arg, char *key) +const char *get_key_from_ref_arg(const CMPIArgs *args, char *arg, char *key) { CMPIObjectPath *ref = NULL; - - ref = cu_get_ref_arg(args, arg); - if (ref == NULL) - return NULL; - - return cu_get_str_path(ref, key); + const char *val = NULL; + + if (cu_get_ref_arg(args, arg, &ref) != CMPI_RC_OK) + return NULL; + + if (cu_get_str_path(ref, key, &val) != CMPI_RC_OK) + return NULL; + + return val; } bool domain_exists(virConnectPtr conn, const char *name) @@ -316,7 +319,7 @@ bool domain_online(virDomainPtr dom) (info.state == VIR_DOMAIN_RUNNING); } -int parse_id(char *id, +int parse_id(const char *id, char **pfx, char **name) { @@ -350,15 +353,12 @@ bool parse_instanceid(const CMPIObjectPa char **name) { int ret; - char *id = NULL; - - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) + const char *id = NULL; + + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) return false; ret = parse_id(id, pfx, name); - - free(id); if (!ret) return false; diff -r dca4cb9c22da -r dce4d8a89184 libxkutil/misc_util.h --- a/libxkutil/misc_util.h Tue Nov 27 10:34:13 2007 +0100 +++ b/libxkutil/misc_util.h Tue Nov 27 08:03:00 2007 -0800 @@ -83,7 +83,7 @@ CMPIInstance *get_typed_instance(const C /* Parse an OrgID:LocID string into its constituent parts */ int parse_instance_id(char *iid, char **orgid, char **locid); -char *get_key_from_ref_arg(const CMPIArgs *args, char *arg, char *key); +const char *get_key_from_ref_arg(const CMPIArgs *args, char *arg, char *key); bool domain_exists(virConnectPtr conn, const char *name); bool domain_online(virDomainPtr dom); @@ -93,7 +93,7 @@ char *association_prefix(const char *pro char *association_prefix(const char *provider_name); bool match_pn_to_cn(const char *pn, const char *cn); -int parse_id(char *id, char **pfx, char **name); +int parse_id(const char *id, char **pfx, char **name); bool parse_instanceid(const CMPIObjectPath *ref, char **pfx, char **name); bool libvirt_cim_init(void);

Dan Smith

1:28 p.m.

New subject: [PATCH 02 of 18] Fixes to ComputerSystem for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196194865 28800 # Node ID 620fadffdb9642fb379441d5341f89049c634ae7 # Parent dce4d8a891849228a59d3f90b4b2d30dc098fc3a Fixes to ComputerSystem for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r dce4d8a89184 -r 620fadffdb96 src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Tue Nov 27 08:03:00 2007 -0800 +++ b/src/Virt_ComputerSystem.c Tue Nov 27 12:21:05 2007 -0800 @@ -277,7 +277,7 @@ static int instance_from_dom(const CMPIB /* Given a hypervisor connection and a domain name, return an instance */ CMPIInstance *instance_from_name(const CMPIBroker *broker, virConnectPtr conn, - char *name, + const char *name, const CMPIObjectPath *op) { virDomainPtr dom; @@ -376,7 +376,7 @@ static CMPIStatus return_enum_domains(co static CMPIStatus get_domain(const CMPIObjectPath *reference, const CMPIResult *results, - char *name) + const char *name) { CMPIInstance *inst; CMPIStatus s; @@ -440,10 +440,9 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *reference, const char **properties) { - char *name; - - name = cu_get_str_path(reference, "Name"); - if (name == NULL) { + const char *name; + + if (cu_get_str_path(reference, "Name", &name) != CMPI_RC_OK) { CMPIStatus s; CMSetStatusWithChars(_BROKER, &s, @@ -602,7 +601,7 @@ static CMPIStatus state_change_reset(vir return s; } -static CMPIStatus __state_change(char *name, +static CMPIStatus __state_change(const char *name, uint16_t state, const CMPIObjectPath *ref) { @@ -660,7 +659,7 @@ static CMPIStatus state_change(CMPIMetho CMPIStatus s; uint16_t state; int ret; - char *name = NULL; + const char *name = NULL; ret = cu_get_u16_arg(argsin, "RequestedState", &state); if (!ret) { @@ -668,8 +667,7 @@ static CMPIStatus state_change(CMPIMetho goto out; } - name = cu_get_str_path(reference, "Name"); - if (name == NULL) { + if (cu_get_str_path(reference, "Name", &name) != CMPI_RC_OK) { CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED, "Name key not specified"); @@ -679,8 +677,6 @@ static CMPIStatus state_change(CMPIMetho s = __state_change(name, state, reference); out: - free(name); - return s; } diff -r dce4d8a89184 -r 620fadffdb96 src/Virt_ComputerSystem.h --- a/src/Virt_ComputerSystem.h Tue Nov 27 08:03:00 2007 -0800 +++ b/src/Virt_ComputerSystem.h Tue Nov 27 12:21:05 2007 -0800 @@ -34,7 +34,7 @@ */ CMPIInstance *instance_from_name(const CMPIBroker *broker, virConnectPtr conn, - char *name, + const char *name, const CMPIObjectPath *ns);

Dan Smith

1:28 p.m.

New subject: [PATCH 03 of 18] Fixes to Device for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196194889 28800 # Node ID 57656dc04de25a4b9b4986d94f73b42c9db571b9 # Parent 620fadffdb9642fb379441d5341f89049c634ae7 Fixes to Device for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 620fadffdb96 -r 57656dc04de2 src/Virt_Device.c --- a/src/Virt_Device.c Tue Nov 27 12:21:05 2007 -0800 +++ b/src/Virt_Device.c Tue Nov 27 12:21:29 2007 -0800 @@ -470,7 +470,7 @@ CMPIInstance *instance_from_devid(const static CMPIStatus get_device(const CMPIObjectPath *reference, const CMPIResult *results, - char *devid) + const char *devid) { CMPIStatus s; virConnectPtr conn; @@ -525,10 +525,9 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *reference, const char **properties) { - char *devid; - - devid = cu_get_str_path(reference, "DeviceID"); - if (devid == NULL) { + const char *devid; + + if (cu_get_str_path(reference, "DeviceID", &devid) != CMPI_RC_OK) { CMPIStatus s; CMSetStatusWithChars(_BROKER, &s,

Dan Smith

1:28 p.m.

New subject: [PATCH 04 of 18] Fixes to SystemDevice for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196194912 28800 # Node ID 64da04a35e65ed87ee6bd144cdbf5a9d8efd9383 # Parent 57656dc04de25a4b9b4986d94f73b42c9db571b9 Fixes to SystemDevice for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 57656dc04de2 -r 64da04a35e65 src/Virt_SystemDevice.c --- a/src/Virt_SystemDevice.c Tue Nov 27 12:21:29 2007 -0800 +++ b/src/Virt_SystemDevice.c Tue Nov 27 12:21:52 2007 -0800 @@ -158,14 +158,13 @@ static CMPIStatus sys_to_dev(const CMPIO struct std_assoc_info *info, struct inst_list *list) { - char *host = NULL; + const char *host = NULL; CMPIStatus s = {CMPI_RC_OK, NULL}; int ret; ASSOC_MATCH(info->provider_name, CLASSNAME(ref)); - host = cu_get_str_path(ref, "Name"); - if (host == NULL) { + if (cu_get_str_path(ref, "Name", &host) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing Name"); @@ -205,7 +204,7 @@ static CMPIStatus dev_to_sys(const CMPIO struct std_assoc_info *info, struct inst_list *list) { - char *devid = NULL; + const char *devid = NULL; char *host = NULL; char *dev = NULL; CMPIInstance *sys; @@ -213,8 +212,7 @@ static CMPIStatus dev_to_sys(const CMPIO ASSOC_MATCH(info->provider_name, CLASSNAME(ref)); - devid = cu_get_str_path(ref, "DeviceID"); - if (devid == NULL) { + if (cu_get_str_path(ref, "DeviceID", &devid) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing DeviceID"); @@ -242,7 +240,6 @@ static CMPIStatus dev_to_sys(const CMPIO out: free(dev); free(host); - free(devid); return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 05 of 18] Fixes to RASD for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196194946 28800 # Node ID bf7fc8b2c3b68b409052757853b450002bb7e1da # Parent 64da04a35e65ed87ee6bd144cdbf5a9d8efd9383 Fixes to RASD for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 64da04a35e65 -r bf7fc8b2c3b6 src/Virt_RASD.c --- a/src/Virt_RASD.c Tue Nov 27 12:21:52 2007 -0800 +++ b/src/Virt_RASD.c Tue Nov 27 12:22:26 2007 -0800 @@ -247,11 +247,10 @@ static CMPIStatus GetInstance(CMPIInstan { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *inst; - char *id = NULL; + const char *id = NULL; uint16_t type; - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -274,8 +273,6 @@ static CMPIStatus GetInstance(CMPIInstan CMPI_RC_ERR_FAILED, "Unknown instance"); out: - free(id); - return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 06 of 18] Fixes to VSMS for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195058 28800 # Node ID 6e84b22c2d0251897d1f12a97fc79a0cfa2c0db9 # Parent bf7fc8b2c3b68b409052757853b450002bb7e1da Fixes to VSMS for libcmpiutil API change Changed the name of a variable to avoid having to break a line in an ugly way. If this is too much of a violation of the recently defined policy, I'll changed it :) Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r bf7fc8b2c3b6 -r 6e84b22c2d02 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Tue Nov 27 12:22:26 2007 -0800 +++ b/src/Virt_VirtualSystemManagementService.c Tue Nov 27 12:24:18 2007 -0800 @@ -92,12 +92,11 @@ static CMPIStatus define_system_parse_ar struct inst_list *res) { CMPIStatus s = {CMPI_RC_ERR_FAILED, NULL}; - char *sys_str = NULL; + const char *sys_str = NULL; CMPIArray *res_arr; int ret; - sys_str = cu_get_str_arg(argsin, "SystemSettings"); - if (sys_str == NULL) { + if (cu_get_str_arg(argsin, "SystemSettings", &sys_str) != CMPI_RC_OK) { CU_DEBUG("No SystemSettings string argument"); goto out; } @@ -114,8 +113,8 @@ static CMPIStatus define_system_parse_ar goto out; } - res_arr = cu_get_array_arg(argsin, "ResourceSettings"); - if (res_arr == NULL) { + if (cu_get_array_arg(argsin, "ResourceSettings", &res_arr) != + CMPI_RC_OK) { CU_DEBUG("Failed to get array arg"); goto out; } @@ -125,8 +124,6 @@ static CMPIStatus define_system_parse_ar CMSetStatus(&s, CMPI_RC_OK); out: - free(sys_str); - return s; } @@ -135,11 +132,14 @@ static int vssd_to_domain(CMPIInstance * { uint16_t tmp; int ret = 0; + const char *val; + + ret = cu_get_str_prop(inst, "VirtualSystemIdentifier", &val); + if (ret != CMPI_RC_OK) + goto out; free(domain->name); - ret = cu_get_str_prop(inst, "VirtualSystemIdentifier", &domain->name); - if (ret != CMPI_RC_OK) - goto out; + domain->name = strdup(val); ret = cu_get_u16_prop(inst, "AutomaticShutdownAction", &tmp); if (ret != CMPI_RC_OK) @@ -153,15 +153,19 @@ static int vssd_to_domain(CMPIInstance * domain->on_crash = (int)tmp; + ret = cu_get_str_prop(inst, "Bootloader", &val); + if (ret != CMPI_RC_OK) + val = ""; + free(domain->bootloader); - ret = cu_get_str_prop(inst, "Bootloader", &domain->bootloader); + domain->bootloader = strdup(val); + + ret = cu_get_str_prop(inst, "BootloaderArgs", &val); if (ret != CMPI_RC_OK) - domain->bootloader = strdup(""); + val = ""; free(domain->bootloader_args); - ret = cu_get_str_prop(inst, "BootloaderArgs", &domain->bootloader_args); - if (ret != CMPI_RC_OK) - domain->bootloader_args = strdup(""); + domain->bootloader_args = strdup(val); ret = 1; out: @@ -172,7 +176,8 @@ static int rasd_to_vdev(CMPIInstance *in struct virt_device *dev) { uint16_t type; - char *id = NULL; + const char *id = NULL; + const char *val = NULL; char *name = NULL; char *devid = NULL; CMPIObjectPath *op; @@ -196,16 +201,20 @@ static int rasd_to_vdev(CMPIInstance *in free(dev->dev.disk.virtual_dev); dev->dev.disk.virtual_dev = devid; + if (cu_get_str_prop(inst, "Address", &val) != CMPI_RC_OK) + val = "/dev/null"; + free(dev->dev.disk.source); - cu_get_str_prop(inst, "Address", &dev->dev.disk.source); + dev->dev.disk.source = strdup(val); } else if (type == VIRT_DEV_NET) { free(dev->dev.net.mac); dev->dev.net.mac = devid; + if (cu_get_str_prop(inst, "NetworkType", &val) != CMPI_RC_OK) + val = "bridge"; + free(dev->dev.net.type); - cu_get_str_prop(inst, "NetworkType", &dev->dev.net.type); - if (dev->dev.net.type == NULL) - dev->dev.net.type = strdup("bridge"); + dev->dev.net.type = strdup(val); } else if (type == VIRT_DEV_MEM) { cu_get_u64_prop(inst, "VirtualQuantity", &dev->dev.mem.size); cu_get_u64_prop(inst, "Reservation", &dev->dev.mem.size); @@ -215,12 +224,10 @@ static int rasd_to_vdev(CMPIInstance *in dev->dev.mem.maxsize <<= 10; } - free(id); free(name); return 1; err: - free(id); free(name); free(devid); @@ -411,7 +418,7 @@ static CMPIStatus destroy_system(CMPIMet const CMPIArgs *argsin, CMPIArgs *argsout) { - char *dom_name = NULL; + const char *dom_name = NULL; CMPIStatus status = {CMPI_RC_OK, NULL}; CMPIValue rc; CMPIObjectPath *sys; @@ -426,8 +433,7 @@ static CMPIStatus destroy_system(CMPIMet goto error1; } - sys = cu_get_ref_arg(argsin, "AffectedSystem"); - if (sys == NULL) { + if (cu_get_ref_arg(argsin, "AffectedSystem", &sys) != CMPI_RC_OK) { rc.uint32 = IM_RC_FAILED; goto error2; } @@ -455,7 +461,6 @@ static CMPIStatus destroy_system(CMPIMet rc.uint32 = IM_RC_SYS_NOT_FOUND; } - free(dom_name); error2: virConnectClose(conn); error1: @@ -467,7 +472,7 @@ static CMPIStatus update_system_settings CMPIInstance *vssd) { CMPIStatus s; - char *name = NULL; + const char *name = NULL; CMPIrc ret; virConnectPtr conn = NULL; virDomainPtr dom = NULL; @@ -513,7 +518,6 @@ static CMPIStatus update_system_settings out: free(xml); - free(name); virDomainFree(dom); virConnectClose(conn); cleanup_dominfo(&dominfo); @@ -529,11 +533,10 @@ static CMPIStatus mod_system_settings(CM const CMPIArgs *argsin, CMPIArgs *argsout) { - char *inst_str; + const char *inst_str; CMPIInstance *inst; - inst_str = cu_get_str_arg(argsin, "SystemSettings"); - if (inst == NULL) { + if (cu_get_str_arg(argsin, "SystemSettings", &inst_str) != CMPI_RC_OK) { CMPIStatus s; cu_statusf(_BROKER, &s, @@ -878,7 +881,7 @@ static CMPIStatus _update_resource_setti for (i = 0; i < list->cur; i++) { CMPIInstance *inst = list->list[i]; - char *id = NULL; + const char *id = NULL; char *name = NULL; char *devid = NULL; virDomainPtr dom = NULL; @@ -910,7 +913,6 @@ static CMPIStatus _update_resource_setti end: free(name); free(devid); - free(id); virDomainFree(dom); if (s.rc != CMPI_RC_OK) @@ -927,21 +929,20 @@ static CMPIStatus update_resource_settin const CMPIArgs *argsin, resmod_fn func) { - CMPIArray *array; + CMPIArray *arr; CMPIStatus s; struct inst_list list; inst_list_init(&list); - array = cu_get_array_arg(argsin, "ResourceSettings"); - if (array == NULL) { + if (cu_get_array_arg(argsin, "ResourceSettings", &arr) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing ResourceSettings"); goto out; } - parse_str_inst_array(array, NAMESPACE(ref), &list); + parse_str_inst_array(arr, NAMESPACE(ref), &list); s = _update_resource_settings(ref, &list, func); @@ -1052,7 +1053,7 @@ CMPIStatus get_vsms(const CMPIObjectPath CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *inst = NULL; CMPIInstance *host = NULL; - char *val = NULL; + const char *val = NULL; virConnectPtr conn = NULL; *_inst = NULL; @@ -1089,7 +1090,6 @@ CMPIStatus get_vsms(const CMPIObjectPath CMSetProperty(inst, "SystemName", (CMPIValue *)val, CMPI_chars); - free(val); if (cu_get_str_prop(host, "CreationClassName", &val) != CMPI_RC_OK) { cu_statusf(broker, &s, @@ -1100,7 +1100,6 @@ CMPIStatus get_vsms(const CMPIObjectPath CMSetProperty(inst, "SystemCreationClassName", (CMPIValue *)val, CMPI_chars); - free(val); CMSetStatus(&s, CMPI_RC_OK);

Heidi Eckhart

Wednesday, 28 November Wed, 28 Nov

7:17 a.m.

New subject: [PATCH 06 of 18] Fixes to VSMS for libcmpiutil API change

Dan Smith wrote:

...

no problem ;) ... +1 -- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck(a)linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Kaitlin Rupert

11:41 a.m.

New subject: [PATCH 06 of 18] Fixes to VSMS for libcmpiutil API change

Dan Smith wrote:

...

free(dev->dev.net.type); - cu_get_str_prop(inst, "NetworkType", &dev->dev.net.type); - if (dev->dev.net.type == NULL) - dev->dev.net.type = strdup("bridge"); + dev->dev.net.type = strdup(val); } else if (type == VIRT_DEV_MEM) { cu_get_u64_prop(inst, "VirtualQuantity", &dev->dev.mem.size); cu_get_u64_prop(inst, "Reservation", &dev->dev.mem.size); @@ -215,12 +224,10 @@ static int rasd_to_vdev(CMPIInstance *in

I'm not as familiar with the device support as a I should be. =) It looks like your removing support for the NULL case here. Is returning the network type of "bridge" in the NULL case no longer valid? -- Kaitlin Rupert IBM Linux Technology Center karupert(a)us.ibm.com

Dan Smith

11:53 a.m.

New subject: [PATCH 06 of 18] Fixes to VSMS for libcmpiutil API change

...

> - cu_get_str_prop(inst, "NetworkType", &dev->dev.net.type); > - if (dev->dev.net.type == NULL) > - dev->dev.net.type = strdup("bridge"); > + dev->dev.net.type = strdup(val);

KR> I'm not as familiar with the device support as a I should be. =) KR> It looks like your removing support for the NULL case here. Is KR> returning the network type of "bridge" in the NULL case no longer KR> valid? No, that case is still handled. For full context: + if (cu_get_str_prop(inst, "NetworkType", &val) != CMPI_RC_OK) + val = "bridge"; + free(dev->dev.net.type); - cu_get_str_prop(inst, "NetworkType", &dev->dev.net.type); - if (dev->dev.net.type == NULL) - dev->dev.net.type = strdup("bridge"); + dev->dev.net.type = strdup(val); I moved the "not set" (NULL) case above, and set val to "bridge", so that below, I can assume it is a valid const string and strdup it directly. So val either gets the actual type, or "bridge" by the time I get to the strdup. I did the same reordering in disk with the Address field, making "/dev/null" the default if unspecified. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com

Dan Smith

Tuesday, 27 November Tue, 27 Nov

1:28 p.m.

New subject: [PATCH 07 of 18] Fixes to VSMC for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195061 28800 # Node ID 393868c380f668d3d5e765f6503be3eb3eac0a62 # Parent 6e84b22c2d0251897d1f12a97fc79a0cfa2c0db9 Fixes to VSMC for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 6e84b22c2d02 -r 393868c380f6 src/Virt_VirtualSystemManagementCapabilities.c --- a/src/Virt_VirtualSystemManagementCapabilities.c Tue Nov 27 12:24:18 2007 -0800 +++ b/src/Virt_VirtualSystemManagementCapabilities.c Tue Nov 27 12:24:21 2007 -0800 @@ -95,10 +95,9 @@ CMPIStatus get_vsm_cap(const CMPIBroker CMPIStatus s; CMPIObjectPath *op; char *classname = NULL; - char *sys_name = NULL; - - sys_name = cu_get_str_path(ref, "Name"); - if (sys_name == NULL) { + const char *sys_name = NULL; + + if (cu_get_str_path(ref, "Name", &sys_name) != CMPI_RC_OK) { CMSetStatusWithChars(broker, &s, CMPI_RC_ERR_FAILED, "Missing key: Name"); @@ -134,7 +133,6 @@ CMPIStatus get_vsm_cap(const CMPIBroker out: free(classname); - free(sys_name); return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 08 of 18] Fixes to ELEC for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195081 28800 # Node ID 0d30be4ad77c9a476eeee6dcd56c183c7de566a2 # Parent 393868c380f668d3d5e765f6503be3eb3eac0a62 Fixes to ELEC for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 393868c380f6 -r 0d30be4ad77c src/Virt_EnabledLogicalElementCapabilities.c --- a/src/Virt_EnabledLogicalElementCapabilities.c Tue Nov 27 12:24:21 2007 -0800 +++ b/src/Virt_EnabledLogicalElementCapabilities.c Tue Nov 27 12:24:41 2007 -0800 @@ -108,10 +108,9 @@ CMPIStatus get_ele_cap(const CMPIBroker CMPIStatus s; CMPIObjectPath *op; char *classname = NULL; - char *sys_name = NULL; - - sys_name = cu_get_str_path(ref, "Name"); - if (sys_name == NULL) { + const char *sys_name = NULL; + + if (cu_get_str_path(ref, "Name", &sys_name) != CMPI_RC_OK) { CMSetStatusWithChars(broker, &s, CMPI_RC_ERR_FAILED, "Missing key: Name"); @@ -147,7 +146,6 @@ CMPIStatus get_ele_cap(const CMPIBroker out: free(classname); - free(sys_name); return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 09 of 18] Fixes to DevicePool for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195089 28800 # Node ID 7e2a9350d9794c4744ecab188da956434f28ce6c # Parent 0d30be4ad77c9a476eeee6dcd56c183c7de566a2 Fixes to DevicePool for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 0d30be4ad77c -r 7e2a9350d979 src/Virt_DevicePool.c --- a/src/Virt_DevicePool.c Tue Nov 27 12:24:41 2007 -0800 +++ b/src/Virt_DevicePool.c Tue Nov 27 12:24:49 2007 -0800 @@ -730,10 +730,9 @@ static CMPIStatus GetInstance(CMPIInstan CMPIStatus s; CMPIInstance *inst; virConnectPtr conn = NULL; - char *id = NULL; - - id = cu_get_str_path(reference, "InstanceID"); - if (id == NULL) { + const char *id = NULL; + + if (cu_get_str_path(reference, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -756,7 +755,6 @@ static CMPIStatus GetInstance(CMPIInstan out: - free(id); virConnectClose(conn); return s;

Dan Smith

1:28 p.m.

New subject: [PATCH 10 of 18] Fixes to RegisteredProfile for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195095 28800 # Node ID fd942395e4dc888d1eede22d2dae5b99d9eed4eb # Parent 7e2a9350d9794c4744ecab188da956434f28ce6c Fixes to RegisteredProfile for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 7e2a9350d979 -r fd942395e4dc src/Virt_RegisteredProfile.c --- a/src/Virt_RegisteredProfile.c Tue Nov 27 12:24:49 2007 -0800 +++ b/src/Virt_RegisteredProfile.c Tue Nov 27 12:24:55 2007 -0800 @@ -124,11 +124,10 @@ static CMPIStatus get_prof(const CMPIObj { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *instance = NULL; - char* id; + const char* id; int i; - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED, "No InstanceID specified"); @@ -149,9 +148,6 @@ static CMPIStatus get_prof(const CMPIObj CMReturnInstance(results, instance); else CMSetStatus(&s, CMPI_RC_ERR_NOT_FOUND); - - - free(id); return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 11 of 18] Fixes to ECTP for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195105 28800 # Node ID 0bbb737215af97d0704bbbd6e2526f6aec839228 # Parent fd942395e4dc888d1eede22d2dae5b99d9eed4eb Fixes to ECTP for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r fd942395e4dc -r 0bbb737215af src/Virt_ElementConformsToProfile.c --- a/src/Virt_ElementConformsToProfile.c Tue Nov 27 12:24:55 2007 -0800 +++ b/src/Virt_ElementConformsToProfile.c Tue Nov 27 12:25:05 2007 -0800 @@ -101,11 +101,10 @@ static CMPIStatus prof_to_elem(const CMP struct inst_list *list) { CMPIStatus s = {CMPI_RC_OK, NULL}; - char *id; + const char *id; int i; - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED, "No InstanceID specified"); @@ -116,13 +115,11 @@ static CMPIStatus prof_to_elem(const CMP if (STREQ(id, profiles[i]->reg_id)) { s = elem_instances(ref, info, list, profiles[i]); if ((s.rc != CMPI_RC_OK)) - goto error; + goto out; break; } } - - error: - free(id); + out: return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 12 of 18] Changes to SettingsDefineCapabilities for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195114 28800 # Node ID fecc81fd400c780ecb496af2ed9ba17001ff2f05 # Parent 0bbb737215af97d0704bbbd6e2526f6aec839228 Changes to SettingsDefineCapabilities for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 0bbb737215af -r fecc81fd400c src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Tue Nov 27 12:25:05 2007 -0800 +++ b/src/Virt_SettingsDefineCapabilities.c Tue Nov 27 12:25:14 2007 -0800 @@ -499,7 +499,7 @@ static struct sdc_rasd_prop *disk_max(co CMPIStatus *s) { bool ret; - char *inst_id; + const char *inst_id; CMPIrc prop_ret; uint16_t free_space; uint64_t free_64; @@ -507,8 +507,7 @@ static struct sdc_rasd_prop *disk_max(co CMPIInstance *pool_inst; struct sdc_rasd_prop *rasd = NULL; - inst_id = cu_get_str_path(ref, "InstanceID"); - if (inst_id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &inst_id) != CMPI_RC_OK) { cu_statusf(_BROKER, s, CMPI_RC_ERR_FAILED, "Could not get InstanceID."); @@ -558,7 +557,6 @@ static struct sdc_rasd_prop *disk_max(co } out: - free(inst_id); return rasd; }

Dan Smith

1:28 p.m.

New subject: [PATCH 13 of 18] Fixes to ElementCapabilities for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195127 28800 # Node ID 589cad1316a9eb3fe0214cf9515bb72ccaef91f8 # Parent fecc81fd400c780ecb496af2ed9ba17001ff2f05 Fixes to ElementCapabilities for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r fecc81fd400c -r 589cad1316a9 src/Virt_ElementCapabilities.c --- a/src/Virt_ElementCapabilities.c Tue Nov 27 12:25:14 2007 -0800 +++ b/src/Virt_ElementCapabilities.c Tue Nov 27 12:25:27 2007 -0800 @@ -53,8 +53,8 @@ static CMPIStatus sys_to_cap(const CMPIO { CMPIInstance *inst; CMPIrc host_rc; - char *host_name = NULL; - char *sys_name = NULL; + const char *host_name = NULL; + const char *sys_name = NULL; CMPIStatus s = {CMPI_RC_OK, NULL}; s = get_host_cs(_BROKER, ref, &inst); @@ -65,8 +65,7 @@ static CMPIStatus sys_to_cap(const CMPIO if (host_rc != CMPI_RC_OK) goto out; - sys_name = cu_get_str_path(ref, "Name"); - if (!STREQ(sys_name, host_name)) { + if (cu_get_str_path(ref, "Name", &sys_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "System '%s' is not a host system.", sys_name); goto out; @@ -76,7 +75,6 @@ static CMPIStatus sys_to_cap(const CMPIO if (s.rc == CMPI_RC_OK) inst_list_add(list, inst); out: - free(sys_name); return s; } @@ -84,16 +82,15 @@ static CMPIStatus cap_to_sys(const CMPIO struct std_assoc_info *info, struct inst_list *list) { - char *inst_id; + const char *inst_id; char *host; char *device; - char *host_name; + const char *host_name; CMPIrc host_rc; CMPIInstance *inst; CMPIStatus s = {CMPI_RC_OK, NULL}; - inst_id = cu_get_str_path(ref, "InstanceID"); - if (inst_id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &inst_id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Could not get InstanceID."); @@ -123,7 +120,6 @@ static CMPIStatus cap_to_sys(const CMPIO out: free(host); free(device); - free(inst_id); return s; } @@ -145,15 +141,14 @@ static CMPIStatus cap_to_cs(const CMPIOb struct std_assoc_info *info, struct inst_list *list) { - char *inst_id; + const char *inst_id; char *host; char *device; CMPIInstance *inst; virConnectPtr conn; CMPIStatus s = {CMPI_RC_OK, NULL}; - inst_id = cu_get_str_path(ref, "InstanceID"); - if (inst_id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &inst_id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Could not get InstanceID."); @@ -179,7 +174,7 @@ static CMPIStatus cap_to_cs(const CMPIOb error1: free(host); free(device); - free(inst_id); + return s; } @@ -196,13 +191,12 @@ static CMPIStatus pool_to_alloc(const CM struct inst_list *list) { int ret; - char *inst_id; + const char *inst_id; uint16_t type; CMPIInstance *inst = NULL; CMPIStatus s = {CMPI_RC_OK}; - inst_id = cu_get_str_path(ref, "InstanceID"); - if (inst_id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &inst_id) != CMPI_RC_OK) { CMSetStatusWithChars(_BROKER, &s, CMPI_RC_ERR_FAILED, "Could not get InstanceID."); goto out; @@ -225,8 +219,6 @@ static CMPIStatus pool_to_alloc(const CM inst_list_add(list, inst); out: - free(inst_id); - return s; } static CMPIInstance *make_ref(const CMPIObjectPath *ref,

Kaitlin Rupert

Wednesday, 28 November Wed, 28 Nov

11:39 a.m.

New subject: [PATCH 13 of 18] Fixes to ElementCapabilities for libcmpiutil API change

Dan Smith wrote:

...

- sys_name = cu_get_str_path(ref, "Name"); - if (!STREQ(sys_name, host_name)) { + if (cu_get_str_path(ref, "Name", &sys_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "System '%s' is not a host system.", sys_name); goto out;

You're removing the check that confirms the sys_name and host_name match. This looks like it's trying to filter out any potential domains that might be passed in as a ref. Is this check no longer necessary? -- Kaitlin Rupert IBM Linux Technology Center karupert(a)us.ibm.com

Dan Smith

11:57 a.m.

New subject: [PATCH 13 of 18] Fixes to ElementCapabilities for libcmpiutil API change

KR> You're removing the check that confirms the sys_name and host_name KR> match. This looks like it's trying to filter out any potential KR> domains that might be passed in as a ref. Is this check no longer KR> necessary? Oops, looks like my pattern-matching eyeballs got a little cut-happy. We probably need both the "!= CMPI_RC_OK" check and then the STREQ() check below that, right? I'll fix that and repost the set. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com

Dan Smith

Tuesday, 27 November Tue, 27 Nov

1:28 p.m.

New subject: [PATCH 14 of 18] Fixes to VSSDComponent for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195200 28800 # Node ID 4036e27b892058dbe0ea944d74932554b53c2c67 # Parent 589cad1316a9eb3fe0214cf9515bb72ccaef91f8 Fixes to VSSDComponent for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 589cad1316a9 -r 4036e27b8920 src/Virt_VSSDComponent.c --- a/src/Virt_VSSDComponent.c Tue Nov 27 12:25:27 2007 -0800 +++ b/src/Virt_VSSDComponent.c Tue Nov 27 12:26:40 2007 -0800 @@ -117,15 +117,14 @@ static CMPIStatus rasd_to_vssd(const CMP { CMPIStatus s; CMPIInstance *vssd = NULL; - char *id = NULL; + const char *id = NULL; char *host = NULL; char *devid = NULL; int ret; ASSOC_MATCH(info->provider_name, CLASSNAME(ref)); - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -145,7 +144,6 @@ static CMPIStatus rasd_to_vssd(const CMP inst_list_add(list, vssd); out: - free(id); free(host); free(devid);

Dan Smith

1:28 p.m.

New subject: [PATCH 15 of 18] Fixes to SettingsDefineState for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195206 28800 # Node ID eeba8399b981b7539a8103fcf1a196e98476073b # Parent 4036e27b892058dbe0ea944d74932554b53c2c67 Fixes to SettingsDefineState for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 4036e27b8920 -r eeba8399b981 src/Virt_SettingsDefineState.c --- a/src/Virt_SettingsDefineState.c Tue Nov 27 12:26:40 2007 -0800 +++ b/src/Virt_SettingsDefineState.c Tue Nov 27 12:26:46 2007 -0800 @@ -46,7 +46,7 @@ static CMPIInstance *find_rasd(struct in CMPIInstance *inst; for (i = 0; i < list->cur; i++) { - char *id; + const char *id; int ret; inst = list->list[i]; @@ -55,12 +55,8 @@ static CMPIInstance *find_rasd(struct in if (ret != CMPI_RC_OK) continue; - if (STREQ(id, devid)) { - free(id); + if (STREQ(id, devid)) return inst; - } else { - free(id); - } } return NULL; @@ -73,15 +69,14 @@ static CMPIStatus dev_to_rasd(const CMPI CMPIStatus s; CMPIInstance *rasd; struct inst_list rasds; - char *id = NULL; + const char *id = NULL; char *name = NULL; char *devid = NULL; int ret; inst_list_init(&rasds); - id = cu_get_str_path(ref, "DeviceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "DeviceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing DeviceID"); @@ -109,14 +104,13 @@ static CMPIStatus dev_to_rasd(const CMPI CMSetStatus(&s, CMPI_RC_OK); out: - free(id); free(name); free(devid); return s; } -static CMPIInstance *_get_typed_device(char *id, +static CMPIInstance *_get_typed_device(const char *id, int type, const CMPIObjectPath *ref, CMPIStatus *s) @@ -161,13 +155,12 @@ static CMPIStatus rasd_to_dev(const CMPI { CMPIStatus s; CMPIInstance *dev = NULL; - char *id = NULL; + const char *id = NULL; uint16_t type; ASSOC_MATCH(info->provider_name, CLASSNAME(ref)); - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -190,8 +183,6 @@ static CMPIStatus rasd_to_dev(const CMPI CMSetStatus(&s, CMPI_RC_OK); out: - free(id); - return s; } @@ -201,7 +192,7 @@ static CMPIStatus vs_to_vssd(const CMPIO { virConnectPtr conn = NULL; virDomainPtr dom = NULL; - char *name; + const char *name; CMPIInstance *vssd; CMPIStatus s; @@ -209,8 +200,7 @@ static CMPIStatus vs_to_vssd(const CMPIO if (conn == NULL) return s; - name = cu_get_str_path(ref, "Name"); - if (name == NULL) { + if (cu_get_str_path(ref, "Name", &name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing Name property"); @@ -232,7 +222,6 @@ static CMPIStatus vs_to_vssd(const CMPIO CMSetStatus(&s, CMPI_RC_OK); out: - free(name); virDomainFree(dom); virConnectClose(conn); @@ -244,7 +233,7 @@ static CMPIStatus vssd_to_vs(const CMPIO struct std_assoc_info *info, struct inst_list *list) { - char *id = NULL; + const char *id = NULL; char *pfx = NULL; char *name = NULL; int ret; @@ -252,8 +241,7 @@ static CMPIStatus vssd_to_vs(const CMPIO CMPIStatus s; CMPIInstance *cs; - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -284,7 +272,6 @@ static CMPIStatus vssd_to_vs(const CMPIO out: free(name); free(pfx); - free(id); virConnectClose(conn);

Dan Smith

1:28 p.m.

New subject: [PATCH 16 of 18] Fixes to RAFP for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195250 28800 # Node ID 1d3cf993995a4d162e11890a04cfcbf2446c3ca2 # Parent eeba8399b981b7539a8103fcf1a196e98476073b Fixes to RAFP for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r eeba8399b981 -r 1d3cf993995a src/Virt_ResourceAllocationFromPool.c --- a/src/Virt_ResourceAllocationFromPool.c Tue Nov 27 12:26:46 2007 -0800 +++ b/src/Virt_ResourceAllocationFromPool.c Tue Nov 27 12:27:30 2007 -0800 @@ -43,7 +43,7 @@ static CMPIStatus rasd_to_pool(const CMP { CMPIStatus s; uint16_t type; - char *id = NULL; + const char *id = NULL; char *poolid = NULL; virConnectPtr conn = NULL; struct inst_list _list; @@ -58,8 +58,7 @@ static CMPIStatus rasd_to_pool(const CMP goto out; } - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -92,7 +91,6 @@ static CMPIStatus rasd_to_pool(const CMP } out: - free(id); free(poolid); virConnectClose(conn); inst_list_free(&_list); @@ -106,7 +104,7 @@ static int filter_by_pool(struct inst_li { int i; uint16_t type; - char *rasd_id = NULL; + const char *rasd_id = NULL; char *poolid = NULL; for (i = 0; i < src->cur; i++) { @@ -126,8 +124,6 @@ static int filter_by_pool(struct inst_li poolid = pool_member_of(_BROKER, CLASSNAME(op), type, rasd_id); if (STREQ(poolid, _poolid)) inst_list_add(dest, inst); - - free(rasd_id); } return dest->cur; @@ -182,10 +178,9 @@ static CMPIStatus pool_to_rasd(const CMP struct inst_list *list) { CMPIStatus s; - char *poolid; - - poolid = cu_get_str_path(ref, "InstanceID"); - if (poolid == NULL) { + const char *poolid; + + if (cu_get_str_path(ref, "InstanceID", &poolid) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -222,8 +217,6 @@ static CMPIStatus pool_to_rasd(const CMP CMSetStatus(&s, CMPI_RC_OK); out: - free(poolid); - return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 17 of 18] Fixes to EAFP for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195268 28800 # Node ID fa0aa635ce2eb93e8a6f8581056f3d0f40122389 # Parent 1d3cf993995a4d162e11890a04cfcbf2446c3ca2 Fixes to EAFP for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r 1d3cf993995a -r fa0aa635ce2e src/Virt_ElementAllocatedFromPool.c --- a/src/Virt_ElementAllocatedFromPool.c Tue Nov 27 12:27:30 2007 -0800 +++ b/src/Virt_ElementAllocatedFromPool.c Tue Nov 27 12:27:48 2007 -0800 @@ -63,7 +63,7 @@ static CMPIStatus vdev_to_pool(const CMP { CMPIStatus s; uint16_t type; - char *id = NULL; + const char *id = NULL; char *poolid = NULL; virConnectPtr conn = NULL; CMPIInstance *pool = NULL; @@ -76,8 +76,7 @@ static CMPIStatus vdev_to_pool(const CMP goto out; } - id = cu_get_str_path(ref, "DeviceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "DeviceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing DeviceID"); @@ -107,7 +106,6 @@ static CMPIStatus vdev_to_pool(const CMP } out: - free(id); free(poolid); virConnectClose(conn); @@ -124,23 +122,20 @@ static int filter_by_pool(struct inst_li for (i = 0; i < src->cur; i++) { CMPIInstance *inst = src->list[i]; - char *cn = NULL; - char *dev_id = NULL; + const char *cn = NULL; + const char *dev_id = NULL; cu_get_str_prop(inst, "CreationClassName", &cn); cu_get_str_prop(inst, "DeviceID", &dev_id); if ((dev_id == NULL) || (cn == NULL)) - goto end; + continue; printf("Device %hhi:%s", type, dev_id); poolid = pool_member_of(_BROKER, cn, type, dev_id); if (poolid && STREQ(poolid, _poolid)) inst_list_add(dest, inst); - end: - free(dev_id); - free(cn); } return dest->cur; @@ -200,11 +195,10 @@ static CMPIStatus pool_to_vdev(const CMP struct std_assoc_info *info, struct inst_list *list) { - char *poolid; + const char *poolid; CMPIStatus s; - poolid = cu_get_str_path(ref, "InstanceID"); - if (poolid == NULL) { + if (cu_get_str_path(ref, "InstanceID", &poolid) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -244,8 +238,6 @@ static CMPIStatus pool_to_vdev(const CMP CMSetStatus(&s, CMPI_RC_OK); out: - free(poolid); - return s; }

Dan Smith

1:28 p.m.

New subject: [PATCH 18 of 18] Fixes to ElementSettingData for libcmpiutil API change

# HG changeset patch # User Dan Smith <danms(a)us.ibm.com> # Date 1196195285 28800 # Node ID 4a88569d292542741d8e9260111e9b2f4d4dee9f # Parent fa0aa635ce2eb93e8a6f8581056f3d0f40122389 Fixes to ElementSettingData for libcmpiutil API change Signed-off-by: Dan Smith <danms(a)us.ibm.com> diff -r fa0aa635ce2e -r 4a88569d2925 src/Virt_ElementSettingData.c --- a/src/Virt_ElementSettingData.c Tue Nov 27 12:27:48 2007 -0800 +++ b/src/Virt_ElementSettingData.c Tue Nov 27 12:28:05 2007 -0800 @@ -90,13 +90,12 @@ static CMPIStatus rasd_to_rasd(const CMP { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *inst; - char *id = NULL; + const char *id = NULL; uint16_t type; ASSOC_MATCH(info->provider_name, CLASSNAME(ref)); - id = cu_get_str_path(ref, "InstanceID"); - if (id == NULL) { + if (cu_get_str_path(ref, "InstanceID", &id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Missing InstanceID"); @@ -122,8 +121,6 @@ static CMPIStatus rasd_to_rasd(const CMP inst_list_add(list, inst); out: - free(id); - return s; }

Jay Gagnon

3:54 p.m.

Dan Smith wrote:

...

I read through as many as I could force myself to, and they all look fine. Once my build machine is back up I'll try to test a few of mine to make sure they work like they used to. -- -Jay

Daniel Veillard

Wednesday, 28 November Wed, 28 Nov

3:08 a.m.

On Tue, Nov 27, 2007 at 12:28:26PM -0700, Dan Smith wrote:

...

Hum, for normal code we usually rely on valgrind to find leaks in the code, for example by instrumenting test program using the code as part of soem regression tests, but with the way CIM providers are used it looks harder to do this. Or would it be possible to design a minimal C(++) program loading the /usr/lib*/cmpi/ shared libraries exercising them locally ? Maybe this exists already, an in general how can we design regression tests for libvirt-cim sounds something worth spending time on :-) Ideas, opinion, or tools pointer anyone ? Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Heidi Eckhart

5:33 a.m.

Daniel Veillard wrote:

...

On Tue, Nov 27, 2007 at 12:28:26PM -0700, Dan Smith wrote: > This refactors all of the necessary functions in libvirt-cim for the new > API. Things only got cleaner, I think. I also found a substantial number > of memory leaks in the process, which are fundamentally fixed by the new > API which I think is a validation of it as "the right thing to do". All > required changes should have been easily spotted by the compiler, but some > testing would be good. > > Tip: To find the leaks, note every time we convert a variable from > char * to const char * and don't remove a subsequent free() call :) > Hum, for normal code we usually rely on valgrind to find leaks in the code, for example by instrumenting test program using the code as part of soem regression tests, but with the way CIM providers are used it looks harder to do this.

You can start the CIMOM with valgrind (e.g. "valgrind cimserver" or "valgrind sfcbd"), but the output is really hard to read.

...

Or would it be possible to design a minimal C(++) program loading the /usr/lib*/cmpi/ shared libraries exercising them locally ?

Mhh, I suppose in that case we would write another tiny CIMOM.

...

Maybe this exists already, an in general how can we design regression tests for libvirt-cim sounds something worth spending time on :-)

absolutely

...

Ideas, opinion, or tools pointer anyone ? Daniel

-- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck(a)linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Dan Smith

8:35 a.m.

DV> Hum, for normal code we usually rely on valgrind to find leaks in DV> the code, for example by instrumenting test program using the code DV> as part of soem regression tests, but with the way CIM providers DV> are used it looks harder to do this. Right, *much* harder. DV> Or would it be possible to design a minimal C(++) program loading DV> the /usr/lib*/cmpi/ shared libraries exercising them locally ? There is a *ton* of infrastructure that has to be in place to run the providers, which I'm not sure is worth implementing. DV> Maybe this exists already, an in general how can we design DV> regression tests for libvirt-cim sounds something worth spending DV> time on :-) For the most part, I think we've been quite good about this kind of thing. Keeping functions small and using CIMOM-managed memory where we can helps us avoid these types of issues. Maybe we could spend some time running a CIMOM under valgrind to see if we can get some suppression filters tweaked to make the output reasonable. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com

Kaitlin Rupert

11:46 a.m.

Dan Smith wrote:

...

Applied your patches and did a quick test. I sent two comments (see previous mails). I ran into some problems testing SettingsDefineCapabilities due to a possible issue with AllocationCapabilities. I'm looking into that further. Otherwise, things look good. =) -- Kaitlin Rupert IBM Linux Technology Center karupert(a)us.ibm.com

6233

days inactive

6234

days old

cim@lists.libvirt.org

Manage subscription

28 comments

5 participants

tags (0)

participants (5)

Dan Smith
Daniel Veillard
Heidi Eckhart
Jay Gagnon
Kaitlin Rupert

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

[PATCH 00 of 18] Updates for libcmpiutil API change