3:10 a.m.
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1197367712 -3600
# Node ID 8ef14daa017090e8473e1f958bc5e5e584032398
# Parent b5847180d00719ff39f2d451d4e79d4369952ad5
[CU] Merge do_ref() and do_assoc()
The two functions do_ref() and do_assoc() run through
the same logic. The only difference are the returned
instances. By configuring the do_assoc() function with
an additional paramter (ref_rslt) to inidcate the type
of the returned instance(s) - either reference(names) or
associator(names) - the two functions can be merged.
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r b5847180d007 -r 8ef14daa0170 std_association.c
--- a/std_association.c Fri Dec 07 17:15:07 2007 -0500
+++ b/std_association.c Tue Dec 11 11:08:32 2007 +0100
@@ -226,21 +226,23 @@ static CMPIStatus do_assoc(struct std_as
struct std_assoc_info *info,
const CMPIResult *results,
const CMPIObjectPath *ref,
+ bool ref_rslt,
bool names_only)
{
CMPIStatus s = {CMPI_RC_OK, NULL};
struct inst_list list;
struct std_assoc *handler;
-
- inst_list_init(&list);
+ int i;
CU_DEBUG("Getting handler...");
handler = std_assoc_get_handler(ctx, info, ref);
if (handler == NULL) {
CU_DEBUG("No handler found.");
- goto out;
+ return s;
}
CU_DEBUG("Getting handler succeeded.");
+
+ inst_list_init(&list);
CU_DEBUG("Calling handler->handler...");
s = handler->handler(ref, info, &list);
@@ -257,73 +259,48 @@ static CMPIStatus do_assoc(struct std_as
goto out;
}
- s = filter_results(&list,
- NAMESPACE(ref),
- info->result_class,
- ctx->brkr);
- if (s.rc != CMPI_RC_OK) {
- CU_DEBUG("filter_results did not return CMPI_RC_OK.");
- goto out;
- }
-
- if (list.list == NULL) {
- CU_DEBUG("List is empty.");
- goto out;
- } else {
- CU_DEBUG("Returned %u instance(s).", list.cur);
- }
-
- if (names_only)
- cu_return_instance_names(results, &list);
- else
- cu_return_instances(results, &list);
-
- out:
- inst_list_free(&list);
-
- return s;
-}
-
-static CMPIStatus do_ref(struct std_assoc_ctx *ctx,
- struct std_assoc_info *info,
- const CMPIResult *results,
- const CMPIObjectPath *ref,
- bool names_only)
-{
- CMPIStatus s = {CMPI_RC_OK, NULL};
- struct inst_list list;
- struct std_assoc *handler;
- int i;
-
- inst_list_init(&list);
-
- CU_DEBUG("Getting handler...");
- handler = std_assoc_get_handler(ctx, info, ref);
- if (handler == NULL) {
- CU_DEBUG("No handler found.");
- goto out;
- }
- CU_DEBUG("Getting handler succeeded.");
-
- CU_DEBUG("Calling handler->handler...");
- s = handler->handler(ref, info, &list);
- if ((s.rc != CMPI_RC_OK) || (list.list == NULL))
- goto out;
-
- for (i = 0; i < list.cur; i++) {
- CMPIInstance *refinst;
-
- refinst = handler->make_ref(ref, list.list[i], info, handler);
- if (refinst == NULL)
- continue;
-
+ /* Associators and AssociatorNames */
+ if (!ref_rslt) {
+ s = filter_results(&list,
+ NAMESPACE(ref),
+ info->result_class,
+ ctx->brkr);
+ if (s.rc != CMPI_RC_OK) {
+ CU_DEBUG("filter_results did not return CMPI_RC_OK.");
+ goto out;
+ }
+
+ if (list.list == NULL) {
+ CU_DEBUG("List is empty.");
+ goto out;
+ } else {
+ CU_DEBUG("Returned %u instance(s).", list.cur);
+ }
+ }
+
+ /* References and ReferenceNames */
+ if (ref_rslt) {
+ for (i = 0; i < list.cur; i++) {
+ CMPIInstance *refinst;
+
+ refinst = handler->make_ref(ref, list.list[i], info,
handler);
+ if (refinst == NULL)
+ continue;
+
+ if (names_only)
+ cu_return_instance_name(results, refinst);
+ else
+ CMReturnInstance(results, refinst);
+ }
+ }
+ /* Associators and AssociatorNames */
+ else {
if (names_only)
- cu_return_instance_name(results, refinst);
+ cu_return_instance_names(results, &list);
else
- CMReturnInstance(results, refinst);
- }
-
- CMSetStatus(&s, CMPI_RC_OK);
+ cu_return_instances(results, &list);
+ }
+
out:
inst_list_free(&list);
@@ -359,6 +336,7 @@ CMPIStatus stda_AssociatorNames(CMPIAsso
&info,
results,
reference,
+ false,
true);
}
@@ -386,6 +364,7 @@ CMPIStatus stda_Associators(CMPIAssociat
&info,
results,
reference,
+ false,
false);
}
@@ -406,7 +385,12 @@ CMPIStatus stda_ReferenceNames(CMPIAssoc
self->ft->miName,
};
- return do_ref(self->hdl, &info, results, reference, true);
+ return do_assoc(self->hdl,
+ &info,
+ results,
+ reference,
+ true,
+ true);
}
CMPIStatus stda_References(CMPIAssociationMI *self,
@@ -427,7 +411,12 @@ CMPIStatus stda_References(CMPIAssociati
self->ft->miName,
};
- return do_ref(self->hdl, &info, results, reference, false);
+ return do_assoc(self->hdl,
+ &info,
+ results,
+ reference,
+ true,
+ false);
}
/*
10:35 a.m.
Heidi Eckhart wrote:
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1197367712 -3600
# Node ID 8ef14daa017090e8473e1f958bc5e5e584032398
# Parent b5847180d00719ff39f2d451d4e79d4369952ad5
[CU] Merge do_ref() and do_assoc()
Nice change Heidi! I like how this condenses things. I did a quick
test with this patch - the associations and references I tested looked
fine. Everything looked good on the read through as well. +1
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
10:54 a.m.
HE> # HG changeset patch
HE> # User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
HE> # Date 1197367712 -3600
HE> # Node ID 8ef14daa017090e8473e1f958bc5e5e584032398
HE> # Parent b5847180d00719ff39f2d451d4e79d4369952ad5
HE> [CU] Merge do_ref() and do_assoc()
I'm supportive of this change, however, I have a few comments.
HE> + /* Associators and AssociatorNames */
HE> + if (!ref_rslt) {
HE> + s = filter_results(&list,
HE> + NAMESPACE(ref),
HE> + info->result_class,
HE> + ctx->brkr);
HE> + if (s.rc != CMPI_RC_OK) {
HE> + CU_DEBUG("filter_results did not return
CMPI_RC_OK.");
HE> + goto out;
HE> + }
HE> +
HE> + if (list.list == NULL) {
HE> + CU_DEBUG("List is empty.");
HE> + goto out;
HE> + } else {
HE> + CU_DEBUG("Returned %u instance(s).", list.cur);
HE> + }
HE> + }
Adding these additional clauses to do_assoc() makes the function too
long and confusing, IMHO. It was already rather long after recent
additions. Lets take this opportunity to refine it a bit.
I think that we should be able to boil down the (ref_rslt == NULL) and
(ref_rslt != NULL) cases into two small helper functions. Each should
build a result list.
Thus, we can have two lists, one that is the result of the handler,
and one that is to be returned. So, we can clean it up like this:
if (ref_rslt)
ref_case(&handler_result, &to_return, info);
else
assoc_case(&handler_result, &to_return, info);
if (names_only)
cu_return_instance_names(results, &to_return);
else
cu_return_instances(results, &to_return);
The filtering case of associators would be in assoc_case(), the
make_ref() calls would be in ref_case().
HE> + /* References and ReferenceNames */
HE> + if (ref_rslt) {
HE> + for (i = 0; i < list.cur; i++) {
HE> + CMPIInstance *refinst;
HE> +
HE> + refinst = handler->make_ref(ref, list.list[i], info,
handler);
HE> + if (refinst == NULL)
HE> + continue;
HE> +
HE> + if (names_only)
HE> + cu_return_instance_name(results, refinst);
HE> + else
HE> + CMReturnInstance(results, refinst);
HE> + }
HE> + }
HE> + /* Associators and AssociatorNames */
HE> + else {
HE> if (names_only)
HE> - cu_return_instance_name(results, refinst);
HE> + cu_return_instance_names(results, &list);
HE> else
HE> - CMReturnInstance(results, refinst);
HE> - }
HE> -
HE> - CMSetStatus(&s, CMPI_RC_OK);
HE> + cu_return_instances(results, &list);
HE> + }
HE> +
This would then clear up this (IMHO, very confusing) case where you
first have a associators case, then an "if references, else
associators" case right behind it.
Thoughts?
If I have been too confusing in my explanation, I will be glad to take
your patch and post a modified version to illustrate my intent.
Thanks Heidi!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
12:10 p.m.
Dan Smith wrote:
Adding these additional clauses to do_assoc() makes the function too
long and confusing, IMHO. It was already rather long after recent
additions. Lets take this opportunity to refine it a bit.
I think that we should be able to boil down the (ref_rslt == NULL) and
(ref_rslt != NULL) cases into two small helper functions. Each should
build a result list.
Thus, we can have two lists, one that is the result of the handler,
and one that is to be returned. So, we can clean it up like this:
if (ref_rslt)
ref_case(&handler_result, &to_return, info);
else
assoc_case(&handler_result, &to_return, info);
if (names_only)
cu_return_instance_names(results, &to_return);
else
cu_return_instances(results, &to_return);
The filtering case of associators would be in assoc_case(), the
make_ref() calls would be in ref_case().
This makes sense to me, and I think it's a good suggestion. This also
makes it clearer which piece is collecting references and which is
collecting associators.
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
4:03 a.m.
Dan Smith wrote:
Adding these additional clauses to do_assoc() makes the function too
long and confusing, IMHO. It was already rather long after recent
additions. Lets take this opportunity to refine it a bit.
I think that we should be able to boil down the (ref_rslt == NULL) and
(ref_rslt != NULL) cases into two small helper functions. Each should
build a result list.
[...]
This would then clear up this (IMHO, very confusing) case where you
first have a associators case, then an "if references, else
associators" case right behind it.
Thoughts?
Thank you for your feedback. I will rework the patch as suggested
(hopefully got it ;) ) and send out the patch.
Thanks ... Heidi
--
Regards
Heidi Eckhart
Software Engineer
Linux Technology Center - Open Hypervisor
heidieck(a)linux.vnet.ibm.com
**************************************************
IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschaeftsfuehrung: Herbert Kircher
Sitz der Gesellschaft: Boeblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
6219
days inactive
6221
days old
4 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert