2:17 p.m.
During the validate method arguments call, ensure embedded objects have been parsed if the
CIMOM hasn't done so already.
2:17 p.m.
New subject: [PATCH 1 of 2] [CU] Add embedded object parse functionality to std_invokemethod
# HG changeset patch
# User Kaitlin Rupert <karupert(a)us.ibm.com>
# Date 1201551292 28800
# Node ID a6352980c902f4a0e3d60e97b4c2c9bc6c594674
# Parent 2948ecbed2674aee61a994b06468c2ce9c763bb3
[CU] Add embedded object parse functionality to std_invokemethod.
This adds 3 functions:
-check_for_eo(): Determine whether the argument is an embedded object that needs to be
parsed.
-parse_eo_inst_arg(): Calls cu_parse_embedded_instance() to parse the arg.
-parse_eo_array(): Calls parse_eo_inst_arg() for each item in the arry.
-parse_eo_param(): If argument is an array, walks through the array calling
parse_eo_inst_arg(), otherwise call parse_eo_inst_arg() directly.
parse_eo_inst_arg() and parse_eo_param() are based on the EO parsing pieces in
Virt_VirtualSystemManagementService.
The style of these changes matches the style in the rest of the file - return 0 for
failure, return 1 for success.
Signed-off-by: Kaitlin Rupert <karupert(a)us.ibm.com>
diff -r 2948ecbed267 -r a6352980c902 std_invokemethod.c
--- a/std_invokemethod.c Tue Jan 15 12:54:04 2008 +0100
+++ b/std_invokemethod.c Mon Jan 28 12:14:52 2008 -0800
@@ -29,6 +29,154 @@
#include "std_invokemethod.h"
#define STREQ(a,b) (strcmp(a, b) == 0)
+
+static int check_for_eo(CMPIType type, CMPIType exp_type, CMPIType *ret_type)
+{
+ if ((exp_type == CMPI_instance) && (type == CMPI_string)) {
+ *ret_type = CMPI_instance;
+ return 1;
+ }
+
+ if ((exp_type == CMPI_instanceA) && (type == CMPI_stringA)) {
+ *ret_type = CMPI_instanceA;
+ return 1;
+ }
+
+ *ret_type = -1;
+ return 0;
+}
+
+static int parse_eo_inst_arg(CMPIInstance **inst,
+ const CMPIBroker *broker,
+ CMPIString *val,
+ const char *ns,
+ CMPIStatus *s)
+{
+ int ret;
+ const char *str;
+
+ str = CMGetCharPtr(val);
+
+ if (str == NULL) {
+ CMSetStatus(s, CMPI_RC_ERR_INVALID_PARAMETER);
+ CU_DEBUG("Method parameter value is NULL");
+ return 0;
+ }
+
+ ret = cu_parse_embedded_instance(str,
+ broker,
+ ns,
+ inst);
+
+ /* cu_parse_embedded_instance() returns 0 on success */
+ if ((ret != 0) || CMIsNullObject(inst)) {
+ CMSetStatus(s, CMPI_RC_ERR_FAILED);
+ CU_DEBUG("Unable to parse embedded object");
+ return 0;
+ }
+
+ return 1;
+}
+
+static int parse_eo_array(CMPIArray **array,
+ const CMPIBroker *broker,
+ CMPIData data,
+ const char *ns,
+ int count,
+ CMPIStatus *s)
+{
+ int i;
+ int ret;
+
+ if (CMIsNullValue(data) || CMIsNullObject(data.value.array)) {
+ CMSetStatus(s, CMPI_RC_ERR_INVALID_PARAMETER);
+ CU_DEBUG("Method parameter is NULL");
+ return 0;
+ }
+
+ for (i = 0; i < count; i++) {
+ CMPIData item;
+ CMPIInstance *inst;
+
+ item = CMGetArrayElementAt(data.value.array, i, NULL);
+
+ ret = parse_eo_inst_arg(&inst,
+ broker,
+ item.value.string,
+ ns,
+ s);
+
+ if (ret != 1) {
+ CU_DEBUG("Parsing argument type %d failed",
item.type);
+ return 0;
+ }
+
+ CMSetArrayElementAt(*array, i,
+ (CMPIValue *)&inst,
+ CMPI_instance);
+ }
+
+ return 1;
+}
+
+static int parse_eo_param(CMPIArgs **args,
+ const CMPIBroker *broker,
+ CMPIData data,
+ CMPIType type,
+ const char *arg_name,
+ const char *ns,
+ CMPIStatus *s)
+{
+ CMPIArray *new_array;
+ CMPIInstance *inst;
+ int ret;
+ int count;
+
+ if (type == CMPI_instance) {
+ ret = parse_eo_inst_arg(&inst,
+ broker,
+ data.value.string,
+ ns,
+ s);
+
+ if (ret != 1)
+ return 0;
+
+ *s = CMAddArg(*args,
+ arg_name,
+ &(inst),
+ CMPI_instance);
+ } else if (type == CMPI_instanceA) {
+ count = CMGetArrayCount(data.value.array, NULL);
+
+ new_array = CMNewArray(broker, count, CMPI_instance, s);
+
+ ret = parse_eo_array(&new_array,
+ broker,
+ data,
+ ns,
+ count,
+ s);
+ if (ret != 1)
+ return 0;
+
+ *s = CMAddArg(*args,
+ arg_name,
+ &(new_array),
+ CMPI_instanceA);
+ } else {
+ CMSetStatus(s, CMPI_RC_ERR_FAILED);
+ CU_DEBUG("Unable to parse argument type %d", type);
+ return 0;
+ }
+
+ if (s->rc != CMPI_RC_OK) {
+ CU_DEBUG("Unable to update method argument");
+ return 0;
+ }
+
+ return 1;
+}
static int validate_arg_type(struct method_arg *arg,
const CMPIArgs *args,
3:02 p.m.
New subject: [PATCH 1 of 2] [CU] Add embedded object parse functionality to std_invokemethod
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
6:27 p.m.
New subject: [PATCH 1 of 2] [CU] Add embedded object parse functionality to std_invokemethod
Dan Smith wrote:
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?
Yes - this is an excellent suggestion thanks. I looked for something
like CMPI_null, but must have missed it in cmpidt.h.
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> +{
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.
Good call.
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?
Yes, since modify args. This is the new arg element that will
eventually be returned. However, based on the suggestion above, I think
things can be reworked so that we won't need the 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.
I was concerned about setting the CMPIValueState state info correctly,
but if we add just the value of the CMPIData element to the array, then
we don't need to be concerned with the state. Yep, I think it's a good
suggestion.
Thanks!
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com
6:47 p.m.
New subject: [PATCH 1 of 2] [CU] Add embedded object parse functionality to std_invokemethod
KR> Yes, since modify args. This is the new arg element that will
KR> eventually be returned.
Er, the rest of the function only seems to call CMAddArg(*args, ...),
which doesn't necessitate a double pointer. Unless I missed something
like:
*args = CMNewArgs(...);
then I think you're fine.
KR> However, based on the suggestion above, I think things can be
KR> reworked so that we won't need the double-pointer.
Even better :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
7:34 p.m.
New subject: [PATCH 1 of 2] [CU] Add embedded object parse functionality to std_invokemethod
Dan Smith wrote:
KR> Yes, since modify args. This is the new arg element that
will
KR> eventually be returned.
Er, the rest of the function only seems to call CMAddArg(*args, ...),
which doesn't necessitate a double pointer. Unless I missed something
like:
*args = CMNewArgs(...);
then I think you're fine.
KR> However, based on the suggestion above, I think things can be
KR> reworked so that we won't need the double-pointer.
Even better :)
Hmm - I didn't end up fixing this. The call is:
+static int parse_eo_param(CMPIArgs **args,
+ const CMPIBroker *broker,
+ CMPIData data,
+ CMPIType type,
+ const char *arg_name,
+ const char *ns,
+ CMPIStatus *s)
+{
parse_eo_param() updates the args list using CMAddArg().
parse_eo_param() is called by validate_arg_type(). validate_arg_type()
gets called on every arg in the original arg list.
So, to remove the double pointer, you'd either need to pass the value up
from parse_eo_param() to validate_arg_type() and then up again, or you
can just pass the arg list down and modify it as you go.
I took a look at passing the value up, but dealing with a CMPIValue arg
didn't seem as clean. I'm definitely open to suggestions here though,
because I think this is awkward as it is.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com
9:40 a.m.
New subject: [PATCH 1 of 2] [CU] Add embedded object parse functionality to std_invokemethod
Kaitlin Rupert wrote:
Dan Smith wrote:
> KR> Yes, since modify args. This is the new arg element that will
> KR> eventually be returned.
>
> Er, the rest of the function only seems to call CMAddArg(*args, ...),
> which doesn't necessitate a double pointer. Unless I missed something
> like:
> *args = CMNewArgs(...);
>
> then I think you're fine.
>
> KR> However, based on the suggestion above, I think things can be
> KR> reworked so that we won't need the double-pointer.
>
> Even better :)
Hmm - I didn't end up fixing this. The call is:
+static int parse_eo_param(CMPIArgs **args,
+ const CMPIBroker *broker,
+ CMPIData data,
+ CMPIType type,
+ const char *arg_name,
+ const char *ns,
+ CMPIStatus *s)
+{
parse_eo_param() updates the args list using CMAddArg().
parse_eo_param() is called by validate_arg_type(). validate_arg_type()
gets called on every arg in the original arg list.
So, to remove the double pointer, you'd either need to pass the value up
from parse_eo_param() to validate_arg_type() and then up again, or you
can just pass the arg list down and modify it as you go.
I took a look at passing the value up, but dealing with a CMPIValue arg
didn't seem as clean. I'm definitely open to suggestions here though,
because I think this is awkward as it is.
Ah, my mistake on the double-pointer here. For some reason, I was
thinking we'd need the double-pointer so that we could update the arg
list. But we only need to update what the pointer points to, so you're
right - we can remove the double pointer. I'll send out an update.
--
Kaitlin Rupert
IBM Linux Technology Center
kaitlin(a)linux.vnet.ibm.com
2:17 p.m.
New subject: [PATCH 2 of 2] [CU] Add parse_inst_arg() to validate_args() call in std_invokemethod
# HG changeset patch
# User Kaitlin Rupert <karupert(a)us.ibm.com>
# Date 1201551293 28800
# Node ID 21c3975da22cbea8218aca2923cfd68abf293a97
# Parent a6352980c902f4a0e3d60e97b4c2c9bc6c594674
[CU] Add parse_inst_arg() to validate_args() call in std_invokemethod.
Signed-off-by: Kaitlin Rupert <karupert(a)us.ibm.com>
diff -r a6352980c902 -r 21c3975da22c std_invokemethod.c
--- a/std_invokemethod.c Mon Jan 28 12:14:52 2008 -0800
+++ b/std_invokemethod.c Mon Jan 28 12:14:53 2008 -0800
@@ -180,9 +180,14 @@ static int parse_eo_param(CMPIArgs **arg
static int validate_arg_type(struct method_arg *arg,
const CMPIArgs *args,
+ const CMPIBroker *broker,
+ const char *ns,
+ CMPIArgs **new_args,
CMPIStatus *s)
{
CMPIData argdata;
+ CMPIType type;
+ int ret;
argdata = CMGetArg(args, arg->name, s);
if ((s->rc != CMPI_RC_OK) || (CMIsNullValue(argdata))) {
@@ -193,10 +198,33 @@ static int validate_arg_type(struct meth
}
if (argdata.type != arg->type) {
- CMSetStatus(s, CMPI_RC_ERR_TYPE_MISMATCH);
- CU_DEBUG("Method parameter `%s' type check failed",
- arg->name);
- return 0;
+ if (check_for_eo(argdata.type, arg->type, &type) == 1) {
+ ret = parse_eo_param(new_args,
+ broker,
+ argdata,
+ type,
+ arg->name,
+ ns,
+ s);
+
+ if (ret != 1)
+ return 0;
+ } else {
+ CMSetStatus(s, CMPI_RC_ERR_TYPE_MISMATCH);
+ CU_DEBUG("Method parameter `%s' type check
failed",
+ arg->name);
+ return 0;
+ }
+ } else {
+ *s = CMAddArg(*new_args,
+ arg->name,
+ &(argdata.value),
+ argdata.type);
+
+ if (s->rc != CMPI_RC_OK) {
+ CU_DEBUG("Unable to update method argument");
+ return 0;
+ }
}
CU_DEBUG("Method parameter `%s' validated type 0x%x",
@@ -207,19 +235,31 @@ static int validate_arg_type(struct meth
}
static int validate_args(struct method_handler *h,
- const CMPIArgs *args,
+ const CMPIArgs **args,
+ const CMPIObjectPath *ref,
+ const CMPIBroker *broker,
CMPIStatus *s)
{
+ CMPIArgs *new_args;
int i;
+
+ new_args = CMNewArgs(broker, s);
for (i = 0; h->args[i].name; i++) {
int ret;
struct method_arg *arg = &h->args[i];
- ret = validate_arg_type(arg, args, s);
+ ret = validate_arg_type(arg,
+ *args,
+ broker,
+ NAMESPACE(ref),
+ &new_args,
+ s);
if (!ret)
return 0;
}
+
+ *args = new_args;
return 1;
}
@@ -254,7 +294,11 @@ CMPIStatus _std_invokemethod(CMPIMethodM
goto exit;
}
- ret = validate_args(h, argsin, &s);
+ ret = validate_args(h,
+ &argsin,
+ reference,
+ ctx->broker,
+ &s);
if (!ret)
goto exit;
6171
days inactive
6172
days old
7 comments
2 participants
participants (2)
-
Dan Smith -
Kaitlin Rupert