Re: widl [4/4]: Detect conformant arrays of user types correctly.

[Robert Shearman]

Dan Hipschman wrote:
> @@ -1859,6 +1859,7 @@ static int get_struct_type(var_list_t *fields)
>      case RPC_FC_OP:
>      case RPC_FC_CARRAY:
>      case RPC_FC_CVARRAY:
> +    case RPC_FC_BOGUS_ARRAY:
>        has_pointer = 1;
>        break;
>
> @@ -1897,15 +1898,9 @@ static int get_struct_type(var_list_t *fields)
>        /* fallthru - treat it as complex */
>
> /* as soon as we see one of these these members, it's bogus... */
> -    case RPC_FC_IP:
>      case RPC_FC_ENCAPSULATED_UNION:
>      case RPC_FC_NON_ENCAPSULATED_UNION:
> -    case RPC_FC_TRANSMIT_AS:
> -    case RPC_FC_REPRESENT_AS:
> -    case RPC_FC_PAD:
> -    case RPC_FC_EMBEDDED_COMPLEX:
>      case RPC_FC_BOGUS_STRUCT:
> -    case RPC_FC_BOGUS_ARRAY:
>        return RPC_FC_BOGUS_STRUCT;
>      }
>    }

Hi Dan,

What's the reasoning behind this part of the patch? It seems to me like 
most of the cases that you removed from becoming a complex structure are 
actually valid. The hunk further up also confuses me - the presence of a 
complex array shouldn't just make the resulting structure into a pointer 
structure, it should make it into a complex structure, since it can't be 
marshalled solely by using memcpy.

-- 
Rob Shearman

widl [4/4]: Detect conformant arrays of user types correctly.

[Dan Hipschman]

On Thu, Oct 18, 2007 at 04:02:40PM +0100, Robert Shearman wrote:
> Dan Hipschman wrote:
> > @@ -1859,6 +1859,7 @@ static int get_struct_type(var_list_t *fields)
> >     case RPC_FC_OP:
> >     case RPC_FC_CARRAY:
> >     case RPC_FC_CVARRAY:
> > +    case RPC_FC_BOGUS_ARRAY:
> >       has_pointer = 1;
> >       break;
> >
> > @@ -1897,15 +1898,9 @@ static int get_struct_type(var_list_t *fields)
> >       /* fallthru - treat it as complex */
> >
> > /* as soon as we see one of these these members, it's bogus... */
> > -    case RPC_FC_IP:
> >     case RPC_FC_ENCAPSULATED_UNION:
> >     case RPC_FC_NON_ENCAPSULATED_UNION:
> > -    case RPC_FC_TRANSMIT_AS:
> > -    case RPC_FC_REPRESENT_AS:
> > -    case RPC_FC_PAD:
> > -    case RPC_FC_EMBEDDED_COMPLEX:
> >     case RPC_FC_BOGUS_STRUCT:
> > -    case RPC_FC_BOGUS_ARRAY:
> >       return RPC_FC_BOGUS_STRUCT;
> >     }
> >   }
>
> Hi Dan,
>
> What's the reasoning behind this part of the patch? It seems to me like 
> most of the cases that you removed from becoming a complex structure are 
> actually valid. The hunk further up also confuses me - the presence of a 
> complex array shouldn't just make the resulting structure into a pointer 
> structure, it should make it into a complex structure, since it can't be 
> marshalled solely by using memcpy.

Hi.

For the first hunk:  Fields that are actually declared as arrays (with
[] notation) are handled above, so if we get to that part of the code
and find a bogus array, it must be a pointer with a size_is attribute.
For conformant arrays (bogus included) that are declared as pointers,
MIDL treats them like pointers to arrays, instead of just embedded
arrays.  Since the field is listed as a pointer, all we need is PSTRUCT
(or variant thereof).  If I understand the RPC engine correctly, a
PSTRUCT should get memcpy'd initially, but then for each pointer it goes
through and marshals whatever it points to separately and then updates
the value.

For the second hunk:  I moved the bogus array type for the reasons
mentioned above.  The other things there aren't real types.  Pointers to
FC_IP are handled above, and it should be impossible to have an
interface itself as a field in a structure.  FC_PAD and
FC_EMBEDDED_COMPLEX aren't types, they're just format codes, and padding
is handled above anyway. The transmit_as and represent_as types haven't
been implemented yet, and when they are I'm guessing they will be done
similarly to the way user_types are done.

Thanks.