Re: [Libvirt-cim] [PATCH] [RFC] Enhance handling of association's references

HE> # HG changeset patch HE> # User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com&gt; HE> # Date 1195819371 -3600 HE> # Node ID db20c6206fb6decb484035bec81d7c7f2be75eae HE> # Parent bf54de6af2e210bef57d74cf12e4872f6ba2da4f HE> [RFC] Enhance handling of association's references HE> The source and target classnames of std_assoc are now lists, HE> containing all supported classnames. This approach frees the HE> provider from listing all possible combinations as instances of HE> std_assoc. I'm tentatively okay with this approach. We need to make a decision and go with it so that we can freeze (at least to some extent) the libcmpiutil API and make an official code release. HE> @@ -113,15 +119,30 @@ std_assoc_get_handler(const struct std_a HE> std_assoc_get_handler(const struct std_assoc_ctx *ctx, HE> const CMPIObjectPath *ref) HE> { HE> - struct std_assoc *ptr; HE> - int i; HE> + struct std_assoc *hdl = NULL; HE> + char *source_class; HE> + int i, j; Please don't arbitrarily rename and reorder variables like this. It makes it harder to read the diff and is really unnecessary. HE> for (i = 0; ctx->handlers[i]; i++) { HE> - ptr = ctx->handlers[i]; HE> - HE> - if (CMClassPathIsA(ctx->brkr, ref, ptr->source_class, NULL)) HE> - return ptr; HE> - } HE> + hdl = ctx->handlers[i]; HE> + HE> + for (j = 0; hdl->source_class[j]; j++) { HE> + source_class = hdl->source_class[j]; HE> + HE> + if (CMClassPathIsA(ctx->brkr, HE> + ref, HE> + source_class, HE> + NULL)) HE> + break; HE> + HE> + source_class = NULL; HE> + } HE> + if (source_class) HE> + break; HE> + } HE> + HE> + if (hdl) HE> + return hdl; This double-nested loop really needs to have the inner loop broken out as another function (like match_class()). HE> return NULL; HE> } HE> @@ -144,9 +165,6 @@ static CMPIStatus do_assoc(struct std_as HE> handler = std_assoc_get_handler(ctx, ref); HE> if (handler == NULL) { HE> CU_DEBUG("No handler found."); HE> - cu_statusf(ctx->brkr, &s, HE> - CMPI_RC_ERR_FAILED, HE> - "Unable to handle this association"); HE> goto out; HE> } HE> @@ -243,9 +261,7 @@ static CMPIStatus do_ref(struct std_asso HE> handler = std_assoc_get_handler(ctx, ref); HE> if (handler == NULL) { HE> - cu_statusf(ctx->brkr, &s, HE> - CMPI_RC_ERR_FAILED, HE> - "Unable to handle this association"); HE> + CU_DEBUG("No handler found."); HE> goto out; HE> } Why should we not return error in these cases now? Currently, these signal the case of someone trying to do something like: wbemcli ain -ac Xen_SystemDevice http://...:Xen_MemoryPool (i.e. resolving a particular association with an invalid reference) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com