This matches midl behaviour. In
void test1([out,size_is(x)] char str[], [in] int x); void test2([size_is(x), out] char* str, [in] int x);
the parameter to test1 is not NULL-checked, but the one to test2 is. --- tools/widl/client.c | 2 +- tools/widl/proxy.c | 15 ++++++++++----- tools/widl/widltypes.h | 1 + 3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tools/widl/client.c b/tools/widl/client.c index a23e298..0b10eb6 100644 --- a/tools/widl/client.c +++ b/tools/widl/client.c @@ -59,7 +59,7 @@ static void check_pointers(const var_t *func)
LIST_FOR_EACH_ENTRY( var, type_get_function_args(func->type), const var_t, entry ) { - if (is_var_ptr(var) && cant_be_null(var)) + if (is_var_declptr(var) && cant_be_null(var)) { print_client("if (!%s)\n", var->name); print_client("{\n"); diff --git a/tools/widl/proxy.c b/tools/widl/proxy.c index bb16b72..ce07ddc 100644 --- a/tools/widl/proxy.c +++ b/tools/widl/proxy.c @@ -150,6 +150,11 @@ int is_var_ptr(const var_t *v) return is_ptr(v->type); }
+int is_var_declptr(const var_t *v) +{ + return is_declptr(v->type); +} + int cant_be_null(const var_t *v) { /* Search backwards for the most recent pointer attribute. */ @@ -164,10 +169,10 @@ int cant_be_null(const var_t *v) if (is_user_type(type)) return 0;
- if (!attrs && is_ptr(type)) + if (!attrs && is_declptr(type)) { attrs = type->attrs; - type = type_pointer_get_ref(type); + type = get_deref_type(type); }
while (attrs) @@ -180,7 +185,7 @@ int cant_be_null(const var_t *v) if (t == RPC_FC_RP) return 1;
- if (is_ptr(type)) + if (is_declptr(type)) { if (type->type == RPC_FC_FP || type->type == RPC_FC_OP || @@ -188,7 +193,7 @@ int cant_be_null(const var_t *v) return 0;
attrs = type->attrs; - type = type_pointer_get_ref(type); + type = get_deref_type(type); } else attrs = NULL; @@ -229,7 +234,7 @@ static void proxy_check_pointers( const var_list_t *args ) if (!args) return; LIST_FOR_EACH_ENTRY( arg, args, const var_t, entry ) { - if (is_var_ptr(arg) && cant_be_null(arg)) { + if (is_var_declptr(arg) && cant_be_null(arg)) { print_proxy( "if(!%s)\n", arg->name ); indent++; print_proxy( "RpcRaiseException(RPC_X_NULL_REF_POINTER);\n"); diff --git a/tools/widl/widltypes.h b/tools/widl/widltypes.h index fe8c250..6a770c3 100644 --- a/tools/widl/widltypes.h +++ b/tools/widl/widltypes.h @@ -445,6 +445,7 @@ void clear_all_offsets(void); int is_ptr(const type_t *t); int is_array(const type_t *t); int is_var_ptr(const var_t *v); +int is_var_declptr(const var_t *v); int cant_be_null(const var_t *v); int is_struct(unsigned char tc); int is_union(unsigned char tc);
2009/1/10 Michael Karcher wine@mkarcher.dialup.fu-berlin.de:
This matches midl behaviour. In
void test1([out,size_is(x)] char str[], [in] int x); void test2([size_is(x), out] char* str, [in] int x);
the parameter to test1 is not NULL-checked, but the one to test2 is.
tools/widl/client.c | 2 +- tools/widl/proxy.c | 15 ++++++++++----- tools/widl/widltypes.h | 1 + 3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/tools/widl/client.c b/tools/widl/client.c index a23e298..0b10eb6 100644 --- a/tools/widl/client.c +++ b/tools/widl/client.c @@ -59,7 +59,7 @@ static void check_pointers(const var_t *func)
LIST_FOR_EACH_ENTRY( var, type_get_function_args(func->type), const var_t, entry ) {
if (is_var_ptr(var) && cant_be_null(var))
if (is_var_declptr(var) && cant_be_null(var)) { print_client("if (!%s)\n", var->name); print_client("{\n");diff --git a/tools/widl/proxy.c b/tools/widl/proxy.c index bb16b72..ce07ddc 100644 --- a/tools/widl/proxy.c +++ b/tools/widl/proxy.c @@ -150,6 +150,11 @@ int is_var_ptr(const var_t *v) return is_ptr(v->type); }
+int is_var_declptr(const var_t *v) +{
- return is_declptr(v->type);
+}
Again, this function doesn't seem to improve readability of the code. I would just move the checking of the type of v into cant_be_null.
int cant_be_null(const var_t *v) { /* Search backwards for the most recent pointer attribute. */ @@ -164,10 +169,10 @@ int cant_be_null(const var_t *v) if (is_user_type(type)) return 0;
- if (!attrs && is_ptr(type))
- if (!attrs && is_declptr(type)) { attrs = type->attrs;
- type = type_pointer_get_ref(type);
type = get_deref_type(type); }
while (attrs)
@@ -180,7 +185,7 @@ int cant_be_null(const var_t *v) if (t == RPC_FC_RP) return 1;
- if (is_ptr(type))
- if (is_declptr(type)) { if (type->type == RPC_FC_FP || type->type == RPC_FC_OP ||
@@ -188,7 +193,7 @@ int cant_be_null(const var_t *v) return 0;
attrs = type->attrs;
type = type_pointer_get_ref(type);
type = get_deref_type(type);
It would be simpler if you just did: if (is_ptr(type)) fc = type->type; else { <loop looking for pointer type> }
return fc == RPC_FC_RP;
Note: there appears to be a bug in the current code where it walks pointers looking for the pointer attribute, instead of walking just aliases, so that should probably be fixed too.
Am Samstag, den 10.01.2009, 23:10 +0000 schrieb Rob Shearman:
+int is_var_declptr(const var_t *v) +{
- return is_declptr(v->type);
+}
Again, this function doesn't seem to improve readability of the code. I would just move the checking of the type of v into cant_be_null.
I didn't add it for readability, but because we already had is_var_ptr and I suspected that the intention was to abstract over the way the type is stored in var_t. If using var->type in client code is not a problem, I happily remove that function. Moving the check to cant_be_null is a nice idea, I consider renaming the function "needs_null_check" then, as cant_be_null(<integer variable>) == 0 might be surprising to the casual reader, as integers might very well be zero.
Note: there appears to be a bug in the current code where it walks pointers looking for the pointer attribute, instead of walking just aliases, so that should probably be fixed too.
Yeah. That function could really use a rewrite. I suppose I don't have to check at that point that a pointer type attribute is not applied to an alias that is not an pointer at all.
I will try to use all your helpful suggestions to submit a better series tomorrow (and to the right address, of course!)
Regards, Michael Karcher
-
Michael Karcher -
Rob Shearman