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.

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