Re: [Libvirt-cim] [PATCH] Set additional attribute values in Virt_ComputerSystem.c

KR> +static int set_other_id_info(const CMPIBroker *broker, KR> + char *uuid, KR> + const char *prefix, KR> + CMPIInstance *instance) KR> +{ KR> + CMPIStatus s; KR> + CMPIArray *id_info; KR> + char *info[3]; KR> + int count = 3; KR> + char *type = "Virtual System"; KR> + char *model; KR> + int i; KR> + KR> + id_info = CMNewArray(broker, KR> + count, KR> + CMPI_string, KR> + &s); KR> + KR> + if (s.rc != CMPI_RC_OK) KR> + return 0; KR> + KR> + if (asprintf(&model, "%s%s", prefix, type) == 1) KR> + return 0; KR> + KR> + info[0] = uuid; KR> + info[1] = model; KR> + info[2] = type; KR> + KR> + for (i = 0; i < count; i++) { KR> + CMPIString *tmp = CMNewString(broker, info[i], NULL); KR> + CMSetArrayElementAt(id_info, i, KR> + &tmp, KR> + CMPI_string); KR> + } KR> + KR> + CMSetProperty(instance, "OtherIdentifyingInfo", KR> + &id_info, CMPI_stringA); KR> + KR> + return 1; KR> +} This is a style thing, I guess, but wouldn't it be easier to combine this function with the one that sets the descriptions? You could loop through both string arrays at the same time and construct the two CMPIArrays together. The two functions are almost identical, so I think doing them together would add six lines to the above function instead of the 30 it takes for the separate function. KR> static int instance_from_dom(const CMPIBroker *broker, KR> virDomainPtr dom, KR> + const char *prefix, KR> CMPIInstance *instance) KR> { KR> + char *uuid; KR> + KR> if (!set_name_from_dom(dom, instance)) { KR> /* Print trace error */ KR> return 0; KR> } KR> - if (!set_uuid_from_dom(dom, instance)) { KR> + if (!set_uuid_from_dom(dom, instance, &uuid)) { KR> /* Print trace error */ KR> return 0; KR> } KR> @@ -269,7 +347,20 @@ static int instance_from_dom(const CMPIB KR> return 0; Leak UUID here... KR> } KR> + if (!set_other_id_info(broker, uuid, prefix, instance)) { KR> + /* Print trace error */ KR> + free(uuid); KR> + return 0; ...but not here... KR> + } KR> + KR> + if (!set_id_desc(broker, instance)) { KR> + /* Print trace error */ KR> + return 0; ...and then again here. KR> + } KR> + KR> /* More attributes here, of course */ KR> + KR> + free(uuid); KR> return 1; KR> } Can we get either an "err:" target or a return variable and an "out:" target please? :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com