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@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@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); - else12: 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); + } }