3:02 p.m.
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(a)us.ibm.com