[PATCH] Some minor std_association fixes
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1194638786 28800 # Node ID 9e90c48e77195c9b1f7f7f892ba6f43f3d7a027d # Parent 5f84fd6c5cdefb2dfe41e4d8ec6ca750d3e69faa Some minor std_association fixes. Change result_list to tmp_list since the list is freed before filter_results() returns. Initialize CMPIStatus s in do_assoc(). If match class fails, then s is never properly set. This is assuming that it's *not* an error if a user queries an association with a result class that isn't handled by the provider. The CU_DEBUG() call before the match_class() call on info->assoc_class has a typo. The string text prints info->assoc_class, but the value passed in for info->assoc_class is info->result_class. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 5f84fd6c5cde -r 9e90c48e7719 std_association.c --- a/std_association.c Thu Nov 08 11:21:50 2007 -0800 +++ b/std_association.c Fri Nov 09 12:06:26 2007 -0800 @@ -80,16 +80,16 @@ static CMPIStatus filter_results(struct const char *filter_class, const CMPIBroker *broker) { - struct inst_list result_list; + struct inst_list tmp_list; CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIObjectPath *op; int i; - result_list = *list; + tmp_list = *list; inst_list_init(list); - for (i = 0; result_list.list[i] != NULL; i++) { - op = CMGetObjectPath(result_list.list[i], &s); + for (i = 0; tmp_list.list[i] != NULL; i++) { + op = CMGetObjectPath(tmp_list.list[i], &s); if ((s.rc != CMPI_RC_OK) || CMIsNullObject(op)) goto out; @@ -100,11 +100,11 @@ static CMPIStatus filter_results(struct if (!match_op(broker, op, filter_class)) continue; - inst_list_add(list, result_list.list[i]); + inst_list_add(list, tmp_list.list[i]); } out: - inst_list_free(&result_list); + inst_list_free(&tmp_list); return s; } @@ -133,7 +133,7 @@ static CMPIStatus do_assoc(struct std_as bool names_only) { struct inst_list list; - CMPIStatus s; + CMPIStatus s = {CMPI_RC_OK, NULL}; struct std_assoc *handler; bool rc; @@ -168,7 +168,7 @@ static CMPIStatus do_assoc(struct std_as CU_DEBUG("Match_class succeeded.\n"); CU_DEBUG("Calling match_class: \n\tinfo->assoc_class: '%s'\n", - info->result_class); + info->assoc_class); rc = match_class(ctx->brkr, NAMESPACE(ref), info->assoc_class,
KR> Change result_list to tmp_list since the list is freed before KR> filter_results() returns. Thanks :) KR> Initialize CMPIStatus s in do_assoc(). If match class fails, then KR> s is never properly set. This is assuming that it's *not* an KR> error if a user queries an association with a result class that KR> isn't handled by the provider. If they try to pass an invalid reference (i.e. one we don't support an association for) we RETURN_UNSUPPORTED(). Shouldn't we do the same thing if they ask for a result that we don't support a transition to? Rather, shouldn't these two cases behave the same, no matter what that behavior should be? KR> The CU_DEBUG() call before the match_class() call on KR> info->assoc_class has a typo. The string text prints KR> info->assoc_class, but the value passed in for info->assoc_class KR> is info->result_class. That wouldn't be confusing in a debug situation at all, would it? :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
If they try to pass an invalid reference (i.e. one we don't support an association for) we RETURN_UNSUPPORTED(). Shouldn't we do the same thing if they ask for a result that we don't support a transition to? Rather, shouldn't these two cases behave the same, no matter what that behavior should be?
You mean, instead of returning success, we should set the status using cu_statusf(). Returning an error makes sense from a client point of view. If the client thinks an association is going to return a specific instance, and nothing is returned, it won't know the state of the system. If an error is returned, it's much more obvious to the client what has happened. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
KR> You mean, instead of returning success, we should set the status KR> using cu_statusf(). Returning an error makes sense from a client KR> point of view. If the client thinks an association is going to KR> return a specific instance, and nothing is returned, it won't know KR> the state of the system. If an error is returned, it's much more KR> obvious to the client what has happened. Right, if you return OK, but no instances, then it just looks like there is nothing on the other end of your association, which would normally be a transient state. However, asking for a result class that we don't support is not a transient error, it's a real one, and we should signal as such. Thanks for catching this and bringing it into the foreground for the requisite beating :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (2)
-
Dan Smith -
Kaitlin Rupert