+ +static CMPIStatus add_devices_to_list(const CMPIObjectPath *ref, + const char *host, + int type, + struct inst_list *list) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + struct inst_list temp_list; + int i, iret; + + inst_list_init(&temp_list); + + s = enum_devices(_BROKER, ref, host, type, &temp_list); + + if (s.rc != CMPI_RC_OK) + goto out; + + for (i = 0; i < temp_list.cur; i++) { + iret = inst_list_add(list, temp_list.list[i]); + if (!iret) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Not enough space to add device to list"); + goto out; + } + }
Why use a temp list here? enum_devices() calls inst_list_add(), so there's no need to duplicate the inst_list_add() code here. Instead, pass the original inst_list to enum_devices() directly.
+ + out: + inst_list_free(&temp_list); + + return s; +}
static CMPIStatus service_to_cs(const CMPIObjectPath *ref, struct std_assoc_info *info, @@ -41,15 +75,60 @@ { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *instance = NULL; + const char *host = NULL; + int i, num_of_domains;
Most of the rest of the code has one variable declaration per line.
+ if (!match_hypervisor_prefix(ref, info)) - return s; + goto out;
s = get_console_rs(ref, &instance, _BROKER, info->context, true); if (s.rc != CMPI_RC_OK) - return s; + goto out;
s = enum_domains(_BROKER, ref, list); + if (s.rc != CMPI_RC_OK) + goto out; + + num_of_domains = list->cur; + + // For each domain, insert its video and pointer devices into + // the list + for (i = 0; i < num_of_domains; i++) { + s.rc = cu_get_str_prop(list->list[i], "Name", &host); + if (s.rc != CMPI_RC_OK) + goto out; + + s = add_devices_to_list(ref, host, CIM_RES_TYPE_INPUT, list); + if (s.rc != CMPI_RC_OK) + goto out; + + s = add_devices_to_list(ref, host, CIM_RES_TYPE_GRAPHICS, list); + if (s.rc != CMPI_RC_OK) + goto out; + }
You can call enum_domains() with a NULL domain param - it will handle the looping for you. The way you have it hear means the code will execute faster. I usually try to avoid duplicating code when possible, but in this case, the reduction of execution time is a benefit.
+ + out: + return s; +} + +static CMPIStatus validate_cs_or_dev_ref(const CMPIContext *context, + const CMPIObjectPath *ref) +{ + CMPIStatus s = {CMPI_RC_OK, NULL};
There's a spacing issue here - might be a tab instead of 8 spaces.
+ CMPIInstance *inst = NULL; + char* classname; + + classname = class_base_name(CLASSNAME(ref)); + + if (STREQC(classname, "ComputerSystem")) { + s = get_domain_by_ref(_BROKER, ref, &inst); + } else if ( (STREQC(classname, "PointingDevice")) || + (STREQC(classname, "DisplayController")) ) {
Most of the other code uses this style: else if ((STREQC()... No spaces between the two sets of parens. Everything tested out fine - so looks good otherwise. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com