dispinterface/custdata typelib conformance

Zebediah Figura z.figura12 at gmail.com
Fri Aug 7 17:58:04 CDT 2020


On 8/7/20 3:36 PM, Kevin Puetz wrote:
> v3 should hopefully make Marvin happier. Thanks to zf on IRC for
> pointing out he won't understand just a two updated (v2) patches...
> 
> Changes from v1:
> - Fix a misplaced hunk (rebase error in splitting things for review)
>   that caused build failures with patches 07/13 to 12/13 
> 
> Changes from v2:
> - additional fix in widl: set oInst=0 (offset in instance) for union fields
>   (existing bug exposed now that test_dump_typelib tests GetVarDesc)
> - skip verification of struct field VARDESC::oInst when test_tlb
>   is a different syskind than the info[] dump in tests/typelib.c
>   Such differences are real, but plausibly legitimate.
> - Fix a few tabs and // comments that I missed (sorry!)
> 
> Kind of a long series, but with the generated code in tests/typelib.c 
> I found squashing much more made it difficult to keep straight,
> which presumably would make reviewing it hard too.

Generally it's better just to send a few patches at a time (5 is a good
number), in particular just by waiting to send the rest rather than
trying to squash anything.

> 
> Kevin Puetz (13):
>   oleaut32/tests: reformat test_dump_typelib.
>   oleaut32/tests: Fix expect_wstr_acpval(...,NULL).
>   widl: all VARDESC fields of TKIND_UNION should have oInst=0.
>   oleaut32/tests: Cover GetVarDesc in test_dump_typelib.
>   oleaut32/tests: Include [dual] interface in test_dump_typelib
>   oleaut32: Fix rewriting FUNCDESC to FUNC_DISPATCH.
> 
> (reasonable stopping point, if you'd prefer to review/apply in steps)
> At this point VARDESC and FUNCDESC should work correctly
> 
>   oleaut32/tests: Cover Get*CustData in test_dump_typelib.
>   widl: remove duplicate '\n\n' in midl_info_guid.
>   widl: parse attribute custom(guid,expr).
>   widl: write ATTR_CUSTOM into typelib.
>   widl: Allow adding the same custdata GUID multiple times in a typelib.
> 
> (reasonable stopping point, though the widl stuff isn't being tested yet)
> 
>   oleaut32: Fix error handling/reporting in TLB_copy_all_custdata.
>   oleaut32: Load GetVarCustData from MSFT-format typelib.
> 
> (complete!)
> 
>  dlls/oleaut32/tests/test_tlb.idl |   86 +-
>  dlls/oleaut32/tests/typelib.c    | 1867 +++++++++++++++++++++++++++---
>  dlls/oleaut32/typelib.c          |  106 +-
>  tools/widl/parser.l              |    2 +-
>  tools/widl/parser.y              |   18 +
>  tools/widl/widltypes.h           |    7 +
>  tools/widl/write_msft.c          |  101 +-
>  7 files changed, 1978 insertions(+), 209 deletions(-)
> 
> Range-diff:
>  1:  254a899e9f =  1:  2fee51637f oleaut32/tests: reformat test_dump_typelib.
>  2:  9976043e87 =  2:  e4aae946f6 oleaut32/tests: Fix expect_wstr_acpval(...,NULL).
>  3:  60e1f39d8c !  3:  37e95834fb widl: fix uninitialized field in VARDESC.
>     @@ Metadata
>      Author: Kevin Puetz <PuetzKevinA at JohnDeere.com>
>      
>       ## Commit message ##
>     -    widl: fix uninitialized field in VARDESC.
>     +    widl: all VARDESC fields of TKIND_UNION should have oInst=0.
>     +
>     +    Also fix uninitialized value in this field for TKIND_DISPATCH.
>      
>          Signed-off-by: Kevin Puetz <PuetzKevinA at JohnDeere.com>
>          ---
>     -    This was just writing the 0x55555555 from xmalloc
>     +    TKIND_UNION previously wrote var_datawidth like TKIND_STRUCT does.
>     +    But a union's datawidth is the max of the fields, not the sum-so-far,
>     +    so this described each field as starting just past the end of the union,
>     +
>     +    TKIND_DISPATCH was just writing the 0x55555555 from xmalloc
>          The field seems to be unused for VAR_DISPATCH,
>     -    but was 0 in the typelibs I had to check (which seems reasonable)
>     +    but was 0 in other (non-widl) typelib files I checked,
>     +    which seems like a reasonable "reserved" value.
>      
>       ## tools/widl/write_msft.c ##
>      @@ tools/widl/write_msft.c: static HRESULT add_var_desc(msft_typeinfo_t *typeinfo, UINT index, var_t* var)
>     +         typeinfo->datawidth += var_datawidth;
>     +         break;
>     +     case TKIND_UNION:
>     +-        typedata[4] = typeinfo->datawidth;
>     ++        typedata[4] = 0;
>     +         typeinfo->datawidth = max(typeinfo->datawidth, var_datawidth);
>               break;
>           case TKIND_DISPATCH:
>               var_kind = 3; /* VAR_DISPATCH */
>  4:  54ab34c62a !  4:  93cb7fc54a oleaut32/tests: Cover GetVarDesc in test_dump_typelib.
>     @@ dlls/oleaut32/tests/typelib.c: typedef struct _type_info
>      +    var_info vars[20];
>       } type_info;
>       
>     ++static const SYSKIND info_syskind = SYS_WIN32;
>       static const type_info info[] = {
>     -@@ dlls/oleaut32/tests/typelib.c: static const type_info info[] = {
>     + /*** Autogenerated data. Do not edit, change the generator above instead. ***/
>     + {
>         "g",
>         "{b14b6bb5-904e-4ff9-b247-bd361f7a0001}",
>         /*kind*/ TKIND_RECORD, /*flags*/ 0, /*align*/ TYPE_ALIGNMENT(struct g), /*size*/ sizeof(struct g),
>     @@ dlls/oleaut32/tests/typelib.c: static const type_info info[] = {
>      +    },
>      +    {
>      +      /*id*/ 0x40000001, /*name*/ "f2", /*flags*/ 0, /*kind*/ VAR_PERINSTANCE,
>     -+      { .oInst = 4 },
>     ++      { .oInst = 0 },
>      +      {VT_PTR, -1, PARAMFLAG_NONE}, /* ret */
>      +    },
>      +  },
>     @@ dlls/oleaut32/tests/typelib.c: static const type_info info[] = {
>      +    },
>      +    {
>      +      /*id*/ 0x40000001, /*name*/ "ff2", /*flags*/ 0, /*kind*/ VAR_PERINSTANCE,
>     -+      { .oInst = 4 },
>     ++      { .oInst = 0 },
>      +      {VT_PTR, -1, PARAMFLAG_NONE}, /* ret */
>      +    },
>      +  },
>     @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>      +    int iface, func, var;
>           VARIANT v;
>           HRESULT hr;
>     ++    TLIBATTR *libattr;
>       
>     +     ole_check(LoadTypeLibEx(name, REGKIND_NONE, &typelib));
>     ++
>     ++    ole_check(ITypeLib_GetLibAttr(typelib, &libattr));
>     ++    if(libattr->syskind != info_syskind) {
>     ++        /* struct VARDESC::oInst may vary from changes in sizeof(void *) affecting the offset of later fields*/
>     ++        skip("ignoring VARDESC::oInst, (libattr->syskind expected %d got %d)\n", info_syskind, libattr->syskind);
>     ++    }
>     ++
>     +     expect_eq(ITypeLib_GetTypeInfoCount(typelib), ticount, UINT, "%d");
>     +     for (iface = 0; iface < ticount; iface++)
>     +     {
>      @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>               expect_int(MAKELONG(typeattr->wMinorVerNum, typeattr->wMajorVerNum), ti->version);
>               expect_int(typeattr->cbSizeVft, ti->cbSizeVft * sizeof(void*));
>     @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>      +            expect_null(desc->lpstrSchema); /* Reserved */
>      +            expect_int(desc->wVarFlags, var_info->wVarFlags);
>      +            expect_int(desc->varkind, var_info->varkind);
>     -+            if(desc->varkind == VAR_PERINSTANCE) {
>     -+                expect_int(desc->DUMMYUNIONNAME.oInst, var_info->DUMMYUNIONNAME.oInst);
>     ++            if (desc->varkind == VAR_PERINSTANCE) {
>     ++                /* oInst depends on preceding field data sizes (except for unions),
>     ++                 * so it may not be valid to expect it to match info[] on other platforms */
>     ++                if ((libattr->syskind == info_syskind) || (typeattr->typekind == TKIND_UNION)) {
>     ++                    expect_int(desc->DUMMYUNIONNAME.oInst, var_info->DUMMYUNIONNAME.oInst);
>     ++                }
>      +            } else if(desc->varkind == VAR_CONST) {
>      +                check_variant_info(desc->DUMMYUNIONNAME.lpvarValue, &var_info->DUMMYUNIONNAME.varValue);
>      +            } else {
>     @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>               ITypeInfo_ReleaseTypeAttr(typeinfo, typeattr);
>       
>               ITypeInfo2_Release(typeinfo2);
>     +         ITypeInfo_Release(typeinfo);
>     +     }
>     ++    ITypeLib_ReleaseTLibAttr(typelib, libattr);
>     +     ITypeLib_Release(typelib);
>     + }
>     + 
>  5:  5f81559d20 !  5:  9ae38e1a9c oleaut32/tests: Include [dual] interface in test_dump_typelib
>     @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>                      "  \"%s\",\n", dump_string(name));
>       
>               OLE_CHECK(ITypeLib_GetTypeInfo(lib, i, &info));
>     -+	if(hRefType) {
>     -+		ITypeInfo *refInfo;
>     -+		OLE_CHECK(ITypeInfo_GetRefTypeInfo(info, hRefType, &refInfo));
>     -+		ITypeInfo_Release(info);
>     -+		info = refInfo;
>     -+	}
>     ++        if(hRefType) {
>     ++            ITypeInfo *refInfo;
>     ++            OLE_CHECK(ITypeInfo_GetRefTypeInfo(info, hRefType, &refInfo));
>     ++            ITypeInfo_Release(info);
>     ++            info = refInfo;
>     ++        }
>               OLE_CHECK(ITypeInfo_GetTypeAttr(info, &attr));
>       
>               printf("  \"%s\",\n", wine_dbgstr_guid(&attr->guid));
>     @@ dlls/oleaut32/tests/typelib.c: static const type_info info[] = {
>      +    HREFTYPE hRefType = 0;
>           VARIANT v;
>           HRESULT hr;
>     +     TLIBATTR *libattr;
>     +@@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>     +         skip("ignoring VARDESC::oInst, (libattr->syskind expected %d got %d)\n", info_syskind, libattr->syskind);
>     +     }
>       
>     -     ole_check(LoadTypeLibEx(name, REGKIND_NONE, &typelib));
>      -    expect_eq(ITypeLib_GetTypeInfoCount(typelib), ticount, UINT, "%d");
>      -    for (iface = 0; iface < ticount; iface++)
>      +    for (const type_info *ti = info; ti != info + ARRAY_SIZE(info); ++ti)
>     @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>       
>               trace("Interface %s\n", ti->name);
>               ole_check(ITypeLib_GetTypeInfo(typelib, iface, &typeinfo));
>     -+	if(hRefType) {
>     -+		ITypeInfo *refInfo;
>     -+		ole_check(ITypeInfo_GetRefTypeInfo(typeinfo, hRefType, &refInfo));
>     -+		ITypeInfo_Release(typeinfo);
>     -+		typeinfo = refInfo;
>     -+	}
>     ++        if(hRefType) {
>     ++            ITypeInfo *refInfo;
>     ++            ole_check(ITypeInfo_GetRefTypeInfo(typeinfo, hRefType, &refInfo));
>     ++            ITypeInfo_Release(typeinfo);
>     ++            typeinfo = refInfo;
>     ++        }
>               ole_check(ITypeLib_GetDocumentation(typelib, iface, &bstrIfName, NULL, &help_ctx, NULL));
>               expect_wstr_acpval(bstrIfName, ti->name);
>               SysFreeString(bstrIfName);
>     @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>               ITypeInfo_Release(typeinfo);
>           }
>      +    expect_eq(ITypeLib_GetTypeInfoCount(typelib), iface, UINT, "%d");
>     +     ITypeLib_ReleaseTLibAttr(typelib, libattr);
>           ITypeLib_Release(typelib);
>       }
>     - 
>  6:  377c3ca32e =  6:  b38819f3e2 oleaut32: Fix rewriting FUNCDESC to FUNC_DISPATCH.
>  7:  380f9fe194 !  7:  5f22c13651 oleaut32/tests: Cover Get*CustData in test_dump_typelib.
>     @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>               OLE_CHECK(ITypeLib_GetDocumentation(lib, i, &name, NULL, &help_ctx, NULL));
>               printf("{\n"
>      @@ dlls/oleaut32/tests/typelib.c: static void test_dump_typelib(const WCHAR *name)
>     - 		ITypeInfo_Release(info);
>     - 		info = refInfo;
>     - 	}
>     +             ITypeInfo_Release(info);
>     +             info = refInfo;
>     +         }
>      +        OLE_CHECK(ITypeInfo_QueryInterface(info, &IID_ITypeInfo2, (void**)&info2));
>      +
>               OLE_CHECK(ITypeInfo_GetTypeAttr(info, &attr));
>     @@ dlls/oleaut32/tests/typelib.c: static const type_info info[] = {
>           },
>           {
>             /*id*/ 0x40000001, /*name*/ "f2", /*flags*/ 0, /*kind*/ VAR_PERINSTANCE,
>     -       { .oInst = 4 },
>     +       { .oInst = 0 },
>      +      /*#custdata*/ 0, {},
>             {VT_PTR, -1, PARAMFLAG_NONE}, /* ret */
>           },
>     @@ dlls/oleaut32/tests/typelib.c: static const type_info info[] = {
>           },
>           {
>             /*id*/ 0x40000001, /*name*/ "ff2", /*flags*/ 0, /*kind*/ VAR_PERINSTANCE,
>     -       { .oInst = 4 },
>     +       { .oInst = 0 },
>      +      /*#custdata*/ 0, {},
>             {VT_PTR, -1, PARAMFLAG_NONE}, /* ret */
>           },
>  8:  f1c1d50ddf =  8:  1cff646dae widl: remove duplicate '\n\n' in midl_info_guid.
>  9:  4996692943 =  9:  67032edfaf widl: parse attribute custom(guid,expr).
> 10:  ead252cc9e ! 10:  d2bb899aa3 widl: write ATTR_CUSTOM into typelib.
>     @@ Commit message
>      
>       ## tools/widl/write_msft.c ##
>      @@ tools/widl/write_msft.c: static HRESULT set_custdata(msft_typelib_t *typelib, REFGUID guid,
>     + 
>     +     guidoffset = ctl2_alloc_guid(typelib, &guidentry);
>     +     if(vt == VT_BSTR)
>     ++        /* TODO midl appears to share a single reference if the same string is used as custdata in multiple places */
>     +         write_string_value(typelib, &data_out, value);
>     +     else
>     +         write_int_value(typelib, &data_out, vt, *(int*)value);
>     +@@ tools/widl/write_msft.c: static HRESULT set_custdata(msft_typelib_t *typelib, REFGUID guid,
>           return S_OK;
>       }
>       
> 11:  25e45884c5 ! 11:  e9966f7f9f widl: Allow adding the same custdata GUID multiple times in a typelib.
>     @@ tools/widl/write_msft.c: static void write_default_value(msft_typelib_t *typelib
>      +    hash_key = ctl2_hash_guid(guid);
>      +    guidoffset = ctl2_find_guid(typelib, hash_key, guid);
>      +    if(guidoffset == -1) {
>     -+        // add GUID that was not already present
>     ++        /* add GUID that was not already present */
>      +        MSFT_GuidEntry guidentry;
>      +        guidentry.guid = *guid;
>       
>     @@ tools/widl/write_msft.c: static void write_default_value(msft_typelib_t *typelib
>       
>      -    guidoffset = ctl2_alloc_guid(typelib, &guidentry);
>           if(vt == VT_BSTR)
>     +         /* TODO midl appears to share a single reference if the same string is used as custdata in multiple places */
>               write_string_value(typelib, &data_out, value);
>     -     else
> 12:  c81690d967 = 12:  26fa308be5 oleaut32: Fix error handling/reporting in TLB_copy_all_custdata.
> 13:  67037eec6a ! 13:  62ea698002 oleaut32: Load GetVarCustData from MSFT-format typelib.
>     @@ dlls/oleaut32/tests/test_tlb.idl: library Test
>      +		int test_property;
>      +methods:
>      +		[id(1),custom(CUSTDATA_STRLIT,"ITypeInfo2::GetFuncCustData dispinterface method")]
>     -+		// FIXME: if the custom strings were identical, midl would de-duplicate them; widl writes them twice.
>      +		void test_method([in,custom(CUSTDATA_STRLIT,"ITypeInfo2::GetParamCustData test_dispatch::test_method(x)")] int x);
>      +	}
>       }
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200807/8988ba3c/attachment.sig>


More information about the wine-devel mailing list