Re: [6/8] widl: Null-check pointers with [size_is]

[Rob Shearman]

2009/1/10 Michael Karcher :
> 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);
> }
>   else
>     attrs = NULL;

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.