KR> +static int check_for_eo(CMPIType type, CMPIType exp_type, CMPIType *ret_type) KR> +{ KR> + if ((exp_type == CMPI_instance) && (type == CMPI_string)) { KR> + *ret_type = CMPI_instance; KR> + return 1; KR> + } KR> + KR> + if ((exp_type == CMPI_instanceA) && (type == CMPI_stringA)) { KR> + *ret_type = CMPI_instanceA; KR> + return 1; KR> + } KR> + KR> + *ret_type = -1; KR> + return 0; KR> +} Instead of taking three CMPIType arguments and returning an int, would it be cleaner to return a CMPIType, and use CMPI_null to signal a failure? KR> +static int parse_eo_array(CMPIArray **array, KR> + const CMPIBroker *broker, KR> + CMPIData data, KR> + const char *ns, KR> + int count, KR> + CMPIStatus *s) KR> +{ KR> + int i; KR> + int ret; KR> + KR> + if (CMIsNullValue(data) || CMIsNullObject(data.value.array)) { KR> + CMSetStatus(s, CMPI_RC_ERR_INVALID_PARAMETER); KR> + CU_DEBUG("Method parameter is NULL"); KR> + return 0; KR> + } KR> + KR> + for (i = 0; i < count; i++) { KR> + CMPIData item; KR> + CMPIInstance *inst; KR> + KR> + item = CMGetArrayElementAt(data.value.array, i, NULL); KR> + KR> + ret = parse_eo_inst_arg(&inst, KR> + broker, KR> + item.value.string, KR> + ns, KR> + s); KR> + KR> + if (ret != 1) { KR> + CU_DEBUG("Parsing argument type %d failed", item.type); KR> + return 0; KR> + } KR> + KR> + CMSetArrayElementAt(*array, i, KR> + (CMPIValue *)&inst, KR> + CMPI_instance); KR> + } KR> + KR> + return 1; KR> +} It seems to me that it would be less confusing to make the above function's signature look like this: static inst parse_eo_array(CMPIArray *strings_in, CMPIArray **instances_out, const CMPIBroker *broker, const char *ns, CMPIStatus *s); If you do this, then you don't need count, and you're not passing around an ambiguously-typed CMPIData. KR> +static int parse_eo_param(CMPIArgs **args, KR> + const CMPIBroker *broker, KR> + CMPIData data, KR> + CMPIType type, KR> + const char *arg_name, KR> + const char *ns, KR> + CMPIStatus *s) KR> +{ Hmm, do we need args as a double-pointer? KR> + CMPIArray *new_array; KR> + CMPIInstance *inst; KR> + int ret; KR> + int count; KR> + KR> + if (type == CMPI_instance) { KR> + ret = parse_eo_inst_arg(&inst, KR> + broker, KR> + data.value.string, KR> + ns, KR> + s); KR> + KR> + if (ret != 1) KR> + return 0; KR> + KR> + *s = CMAddArg(*args, KR> + arg_name, KR> + &(inst), KR> + CMPI_instance); KR> + } else if (type == CMPI_instanceA) { KR> + count = CMGetArrayCount(data.value.array, NULL); KR> + KR> + new_array = CMNewArray(broker, count, CMPI_instance, s); KR> + KR> + ret = parse_eo_array(&new_array, KR> + broker, KR> + data, KR> + ns, KR> + count, KR> + s); KR> + if (ret != 1) KR> + return 0; KR> + KR> + *s = CMAddArg(*args, KR> + arg_name, KR> + &(new_array), KR> + CMPI_instanceA); I think all of the above could become something simpler, like this: if (type == CMPI_instance) ret = parse_eo_inst_arg(&newdata.value.inst, broker, data.value.string, ns, s); else if (type == CMPI_instanceA) ret = parse_eo_array(data.value.array, &newdata.value.array, broker, ns, s); else /* ERROR */ if (ret != 1) return 0; *s = CMAddArg(*args, args, arg_name, &newdata.value, type); return 1; (with some of the details removed)... Right? Might be nice to arrange the parameters of the two similar calls so they match up in order too. Anyway, nothing major, but I think that some or all of the above could help clean it up a bit. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com