Re: OLEAUT32: add additional condition for creation of interface

[Robert Shearman]

Alex Villací­s Lasso wrote:
> diff -ur wine-0.9.6-cvs/dlls/oleaut32/typelib.c wine-0.9.6-cvs-patch/dlls/oleaut32/typelib.c
> --- wine-0.9.6-cvs/dlls/oleaut32/typelib.c	2006-01-16 16:08:20.000000000 -0500
> +++ wine-0.9.6-cvs-patch/dlls/oleaut32/typelib.c	2006-01-24 21:53:48.000000000 -0500
> @@ -5567,10 +5567,31 @@
>         ITypeInfo_AddRef(*ppTInfo);
>         result = S_OK;
>     }
> -    else if (hRefType == -1 &&
> -	(((ITypeInfoImpl*) This)->TypeAttr.typekind   == TKIND_DISPATCH) &&
> -	(((ITypeInfoImpl*) This)->TypeAttr.wTypeFlags &  TYPEFLAG_FDUAL))
> +    else if (
> +	(
> +		hRefType == -1 &&
> +		(This->TypeAttr.typekind   == TKIND_DISPATCH) &&
> +		(This->TypeAttr.wTypeFlags &  TYPEFLAG_FDUAL)
> +	)
> +	||
> +	(
> +		(This->TypeAttr.typekind   == TKIND_DISPATCH) &&
> +		(This->TypeAttr.wTypeFlags &  TYPEFLAG_FDISPATCHABLE)
> +	)
> +	)
>     {
> +	  /* Report whether we are here because of the DBGRID32.OCX fix */
> +	  if (!(
> +		hRefType == -1 &&
> +		(This->TypeAttr.typekind   == TKIND_DISPATCH) &&
> +		(This->TypeAttr.wTypeFlags &  TYPEFLAG_FDUAL)
> +	       ))
> +	  {
> +		FIXME("ignoring hRefType = %d, TYPEFLAG_FDISPATCHABLE is %s\n", 
> +			hRefType,
> +			(This->TypeAttr.wTypeFlags &  TYPEFLAG_FDISPATCHABLE) ? "set" : "not set");
> +	  }	       
> +
>     /* when we meet a DUAL dispinterface, we must create the interface
>     * version of it.
>     */

This doesn't look right to me. We need tests for this situation that 
will tell us whether we are adding a dirty hack or whether this 
behaviour is correct. I can send you a test suite I have already built 
for typelibs if you want to add to it.
> @@ -5598,8 +5619,47 @@
>       if(pRefType->reference == hRefType)
>           break;
>   }
> -	if(!pRefType)
> -	  FIXME("Can't find pRefType for ref %lx\n", hRefType);
> +	if(!pRefType) {
> +		static const char * descTKIND[TKIND_MAX] = {
> +			"TKIND_ENUM",
> +			"TKIND_RECORD",
> +			"TKIND_MODULE",
> +			"TKIND_INTERFACE",
> +			"TKIND_DISPATCH",
> +			"TKIND_COCLASS",
> +			"TKIND_ALIAS",
> +			"TKIND_UNION",            
> +		};
> +		static const char * descTypeFlags[15] = {
> +			"TYPEFLAG_FAPPOBJECT",
> +			"TYPEFLAG_FCANCREATE",
> +			"TYPEFLAG_FLICENSED",
> +			"TYPEFLAG_FPREDECLID",
> +			"TYPEFLAG_FHIDDEN",
> +			"TYPEFLAG_FCONTROL",
> +			"TYPEFLAG_FDUAL",
> +			"TYPEFLAG_FNONEXTENSIBLE",
> +			"TYPEFLAG_FOLEAUTOMATION",
> +			"TYPEFLAG_FRESTRICTED",
> +			"TYPEFLAG_FAGGREGATABLE",
> +			"TYPEFLAG_FREPLACEABLE",
> +			"TYPEFLAG_FDISPATCHABLE",
> +			"TYPEFLAG_FREVERSEBIND",
> +			"TYPEFLAG_FPROXY",
> +		};
> +		int i;
> +
> +		FIXME("Can't find pRefType for ref %lx\n", hRefType);
> +		FIXME("\ttypekind is %s\n", descTKIND[This->TypeAttr.typekind]);
> +		FIXME("\twTypeFlags are:");
> +		for (i = 0; i < 15; i++) {
> +			if (This->TypeAttr.wTypeFlags & (1 << i))
> +			FIXME(" %s", descTypeFlags[i]);
> +		}
> +		if (This->TypeAttr.wTypeFlags & 0xFFFF8000)
> +			FIXME(" %08x", This->TypeAttr.wTypeFlags & 0xFFFF8000);
> +		FIXME("\n");
> +	}
>   if(pRefType && hRefType != -1) {
>             ITypeLib *pTLib = NULL;

This clutters the code too much. There are already functions for dumping 
the typelib that would work here instead, although I think they don't 
use the FIXME channel.

-- 
Rob Shearman

OLEAUT32: add additional condition for creation of interface

[Alex Villací­s Lasso]

Robert Shearman wrote:
> Alex Villací­s Lasso wrote:
> > diff -ur wine-0.9.6-cvs/dlls/oleaut32/typelib.c 
> > wine-0.9.6-cvs-patch/dlls/oleaut32/typelib.c
> > --- wine-0.9.6-cvs/dlls/oleaut32/typelib.c    2006-01-16 
> > 16:08:20.000000000 -0500
> > +++ wine-0.9.6-cvs-patch/dlls/oleaut32/typelib.c    2006-01-24 
> > 21:53:48.000000000 -0500
> > @@ -5567,10 +5567,31 @@
> >         ITypeInfo_AddRef(*ppTInfo);
> >         result = S_OK;
> >     }
> > -    else if (hRefType == -1 &&
> > -    (((ITypeInfoImpl*) This)->TypeAttr.typekind   == TKIND_DISPATCH) &&
> > -    (((ITypeInfoImpl*) This)->TypeAttr.wTypeFlags &  TYPEFLAG_FDUAL))
> > +    else if (
> > +    (
> > +        hRefType == -1 &&
> > +        (This->TypeAttr.typekind   == TKIND_DISPATCH) &&
> > +        (This->TypeAttr.wTypeFlags &  TYPEFLAG_FDUAL)
> > +    )
> > +    ||
> > +    (
> > +        (This->TypeAttr.typekind   == TKIND_DISPATCH) &&
> > +        (This->TypeAttr.wTypeFlags &  TYPEFLAG_FDISPATCHABLE)
> > +    )
> > +    )
> >     {
> > +      /* Report whether we are here because of the DBGRID32.OCX fix */
> > +      if (!(
> > +        hRefType == -1 &&
> > +        (This->TypeAttr.typekind   == TKIND_DISPATCH) &&
> > +        (This->TypeAttr.wTypeFlags &  TYPEFLAG_FDUAL)
> > +           ))
> > +      {
> > +        FIXME("ignoring hRefType = %d, TYPEFLAG_FDISPATCHABLE is 
> > %s\n", +            hRefType,
> > +            (This->TypeAttr.wTypeFlags &  TYPEFLAG_FDISPATCHABLE) ? 
> > "set" : "not set");
> > +      }           +
> >       /* when we meet a DUAL dispinterface, we must create the interface
> >       * version of it.
> >       */
>
> This doesn't look right to me. We need tests for this situation that 
> will tell us whether we are adding a dirty hack or whether this 
> behaviour is correct. I can send you a test suite I have already built 
> for typelibs if you want to add to it.

Please do so. I do not think this is right either (even if DBGRID32.OCX 
runs with it), and I would especially like to see cases where this patch 
causes regressions. Maybe your test suite shows some of them already.
> > @@ -5598,8 +5619,47 @@
> >         if(pRefType->reference == hRefType)
> >             break;
> >     }
> > -    if(!pRefType)
> > -      FIXME("Can't find pRefType for ref %lx\n", hRefType);
> > +    if(!pRefType) {
> > +        static const char * descTKIND[TKIND_MAX] = {
> > +            "TKIND_ENUM",
> > +            "TKIND_RECORD",
> > +            "TKIND_MODULE",
> > +            "TKIND_INTERFACE",
> > +            "TKIND_DISPATCH",
> > +            "TKIND_COCLASS",
> > +            "TKIND_ALIAS",
> > +            "TKIND_UNION",            +        };
> > +        static const char * descTypeFlags[15] = {
> > +            "TYPEFLAG_FAPPOBJECT",
> > +            "TYPEFLAG_FCANCREATE",
> > +            "TYPEFLAG_FLICENSED",
> > +            "TYPEFLAG_FPREDECLID",
> > +            "TYPEFLAG_FHIDDEN",
> > +            "TYPEFLAG_FCONTROL",
> > +            "TYPEFLAG_FDUAL",
> > +            "TYPEFLAG_FNONEXTENSIBLE",
> > +            "TYPEFLAG_FOLEAUTOMATION",
> > +            "TYPEFLAG_FRESTRICTED",
> > +            "TYPEFLAG_FAGGREGATABLE",
> > +            "TYPEFLAG_FREPLACEABLE",
> > +            "TYPEFLAG_FDISPATCHABLE",
> > +            "TYPEFLAG_FREVERSEBIND",
> > +            "TYPEFLAG_FPROXY",
> > +        };
> > +        int i;
> > +
> > +        FIXME("Can't find pRefType for ref %lx\n", hRefType);
> > +        FIXME("\ttypekind is %s\n", 
> > descTKIND[This->TypeAttr.typekind]);
> > +        FIXME("\twTypeFlags are:");
> > +        for (i = 0; i < 15; i++) {
> > +            if (This->TypeAttr.wTypeFlags & (1 << i))
> > +            FIXME(" %s", descTypeFlags[i]);
> > +        }
> > +        if (This->TypeAttr.wTypeFlags & 0xFFFF8000)
> > +            FIXME(" %08x", This->TypeAttr.wTypeFlags & 0xFFFF8000);
> > +        FIXME("\n");
> > +    }
> >     if(pRefType && hRefType != -1) {
> >             ITypeLib *pTLib = NULL;
>
> This clutters the code too much. There are already functions for 
> dumping the typelib that would work here instead, although I think 
> they don't use the FIXME channel.

So, which are they? I would like especially to dump the typelib of 
DBGRID32.OCX and compare it with other (working) ActiveX controls, to 
look for differences.

Alex Villacís Lasso