[PATCH v2 0/4] MR7203: dlls/oleaut32: Cleanups
The intention is to have ICreateTypeLib2_fnDeleteTypeInfo() fully implemented such that Cashbooks can run on Linux. I believe this should fix a number of other Windows apps that lean heavily on oleaut32.dll as well. -- v2: dlls/oleaut32: Add DeleteTypeInfo() base-cases in typelib.c https://gitlab.winehq.org/wine/wine/-/merge_requests/7203
From: Edward O'Callaghan <edward(a)antitrust.cc> Signed-off-by: Edward O'Callaghan <edward(a)antitrust.cc> --- dlls/oleaut32/typelib.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index f421a76fe95..1841fdc88cc 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -3618,19 +3618,14 @@ static ITypeLib2* ITypeLib2_Constructor_MSFT(LPVOID pLib, DWORD dwTLBLength) pTypeLibImpl->dispatch_href = tlbHeader.dispatchpos; - /* type infos */ + /* Allocate and fill typeinfos array. */ if(tlbHeader.nrtypeinfos >= 0 ) { - ITypeInfoImpl **ppTI; - - ppTI = pTypeLibImpl->typeinfos = calloc(tlbHeader.nrtypeinfos, sizeof(ITypeInfoImpl*)); - + pTypeLibImpl->typeinfos = calloc(tlbHeader.nrtypeinfos, sizeof(ITypeInfoImpl*)); for(i = 0; i < tlbHeader.nrtypeinfos; i++) { - *ppTI = MSFT_DoTypeInfo(&cx, i, pTypeLibImpl); - - ++ppTI; - (pTypeLibImpl->TypeInfoCount)++; + pTypeLibImpl->typeinfos[i] = MSFT_DoTypeInfo(&cx, i, pTypeLibImpl); + pTypeLibImpl->TypeInfoCount++; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7203
From: Edward O'Callaghan <edward(a)antitrust.cc> There are a number of 'index' fields at different levels of structs. This can be extremely confusing not to mention there actual intended purpose. Clarify the precise purpose of the pIndex field of ITypeInfoImpl and collect like terms. Signed-off-by: Edward O'Callaghan <edward(a)antitrust.cc> --- dlls/oleaut32/typelib.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index 1841fdc88cc..3c9c8460d4a 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -1216,8 +1216,10 @@ typedef struct tagITypeInfoImpl TYPEATTR typeattr; TYPEDESC *tdescAlias; - ITypeLibImpl * pTypeLib; /* back pointer to typelib */ - int index; /* index in this typelib; */ + /* pIndex and pTypeLib are required by ITypeInfo::GetContainingTypeLib */ + UINT pIndex; /* index of this typelibimpl */ + ITypeLibImpl * pTypeLib; /* back pointer to typelib */ + HREFTYPE hreftype; /* hreftype for app object binding */ /* type libs seem to store the doc strings in ascii * so why should we do it in unicode? @@ -1542,7 +1544,7 @@ static void dump_TypeInfo(const ITypeInfoImpl * pty) TRACE("kind:%s\n", typekind_desc[pty->typeattr.typekind]); TRACE("fct:%u var:%u impl:%u\n", pty->typeattr.cFuncs, pty->typeattr.cVars, pty->typeattr.cImplTypes); TRACE("wTypeFlags: 0x%04x\n", pty->typeattr.wTypeFlags); - TRACE("parent tlb:%p index in TLB:%u\n",pty->pTypeLib, pty->index); + TRACE("parent tlb:%p index in TLB:%u\n",pty->pTypeLib, pty->pIndex); if (pty->typeattr.typekind == TKIND_MODULE) TRACE("dllname:%s\n", debugstr_w(TLB_get_bstr(pty->DllName))); if (TRACE_ON(ole)) dump_TLBFuncDesc(pty->funcdescs, pty->typeattr.cFuncs); @@ -2666,9 +2668,9 @@ static ITypeInfoImpl * MSFT_DoTypeInfo( MSFT_ReadLEDWords(&tiBase, sizeof(tiBase) ,pcx , pcx->pTblDir->pTypeInfoTab.offset+count*sizeof(tiBase)); -/* this is where we are coming from */ + /* set the parent ITypeLibImpl of this ITypeInfoImpl */ + ptiRet->pIndex = count; ptiRet->pTypeLib = pLibInfo; - ptiRet->index=count; ptiRet->guid = MSFT_ReadGuid(tiBase.posguid, pcx); ptiRet->typeattr.lcid = pLibInfo->set_lcid; /* FIXME: correct? */ @@ -4557,8 +4559,8 @@ static ITypeLib2* ITypeLib2_Constructor_SLTG(LPVOID pLib, DWORD dwTLBLength) pTIHeader->res06, pTIHeader->res0e, pTIHeader->res16, pTIHeader->res1e); *ppTypeInfoImpl = ITypeInfoImpl_Constructor(); + (*ppTypeInfoImpl)->pIndex = i; (*ppTypeInfoImpl)->pTypeLib = pTypeLibImpl; - (*ppTypeInfoImpl)->index = i; (*ppTypeInfoImpl)->Name = SLTG_ReadName(pNameTable, pOtherTypeInfoBlks[i].name_offs, pTypeLibImpl); (*ppTypeInfoImpl)->dwHelpContext = pOtherTypeInfoBlks[i].helpcontext; (*ppTypeInfoImpl)->guid = TLB_append_guid(&pTypeLibImpl->guid_list, &pOtherTypeInfoBlks[i].uuid, 2); @@ -8023,13 +8025,13 @@ static HRESULT WINAPI ITypeInfo_fnGetMops( ITypeInfo2 *iface, MEMBERID memid, BS * within that type library. */ static HRESULT WINAPI ITypeInfo_fnGetContainingTypeLib( ITypeInfo2 *iface, - ITypeLib * *ppTLib, UINT *pIndex) + ITypeLib **ppTLib, UINT *pIndex) { ITypeInfoImpl *This = impl_from_ITypeInfo2(iface); /* If a pointer is null, we simply ignore it, the ATL in particular passes pIndex as 0 */ if (pIndex) { - *pIndex=This->index; + *pIndex = This->pIndex; TRACE("returning pIndex=%d\n", *pIndex); } @@ -8562,8 +8564,8 @@ HRESULT WINAPI CreateDispTypeInfo( pTypeLibImpl->typeinfos = calloc(pTypeLibImpl->TypeInfoCount, sizeof(ITypeInfoImpl*)); pTIIface = pTypeLibImpl->typeinfos[0] = ITypeInfoImpl_Constructor(); + pTIIface->pIndex = 0; pTIIface->pTypeLib = pTypeLibImpl; - pTIIface->index = 0; pTIIface->Name = NULL; pTIIface->dwHelpContext = -1; pTIIface->guid = NULL; @@ -8616,8 +8618,8 @@ HRESULT WINAPI CreateDispTypeInfo( dump_TypeInfo(pTIIface); pTIClass = pTypeLibImpl->typeinfos[1] = ITypeInfoImpl_Constructor(); + pTIClass->pIndex = 1; pTIClass->pTypeLib = pTypeLibImpl; - pTIClass->index = 1; pTIClass->Name = NULL; pTIClass->dwHelpContext = -1; pTIClass->guid = NULL; @@ -8861,12 +8863,14 @@ static HRESULT WINAPI ICreateTypeLib2_fnCreateTypeInfo(ICreateTypeLib2 *iface, return TYPE_E_NAMECONFLICT; This->typeinfos = realloc(This->typeinfos, sizeof(ITypeInfoImpl*) * (This->TypeInfoCount + 1)); + This->TypeInfoCount++; - info = This->typeinfos[This->TypeInfoCount] = ITypeInfoImpl_Constructor(); + info = This->typeinfos[This->TypeInfoCount - 1] = ITypeInfoImpl_Constructor(); + info->pIndex = This->TypeInfoCount - 1; info->pTypeLib = This; + info->Name = TLB_append_str(&This->name_list, name); - info->index = This->TypeInfoCount; info->typeattr.typekind = kind; info->typeattr.cbAlignment = 4; @@ -8900,9 +8904,7 @@ static HRESULT WINAPI ICreateTypeLib2_fnCreateTypeInfo(ICreateTypeLib2 *iface, return hres; } - info->hreftype = info->index * sizeof(MSFT_TypeInfoBase); - - ++This->TypeInfoCount; + info->hreftype = info->pIndex * sizeof(MSFT_TypeInfoBase); return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7203
From: Edward O'Callaghan <edward(a)antitrust.cc> Also avoid mixing up types, keep to the canonical varient. Signed-off-by: Edward O'Callaghan <edward(a)antitrust.cc> --- dlls/oleaut32/typelib.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index 3c9c8460d4a..226d2f4b417 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -1090,7 +1090,7 @@ typedef struct tagITypeLibImpl const TLBString *HelpFile; const TLBString *HelpStringDll; DWORD dwHelpContext; - int TypeInfoCount; /* nr of typeinfo's in librarry */ + UINT TypeInfoCount; /* nr of typeinfo's in librarry */ struct tagITypeInfoImpl **typeinfos; struct list custdata_list; struct list implib_list; @@ -1716,14 +1716,14 @@ static inline TLBCustData *TLB_get_custdata_by_guid(const struct list *custdata_ return NULL; } -static inline ITypeInfoImpl *TLB_get_typeinfo_by_name(ITypeLibImpl *typelib, const OLECHAR *name) +static inline ITypeInfoImpl *TLB_get_typeinfo_by_name(ITypeLibImpl *typelib, const OLECHAR *name, UINT *index) { - int i; - - for (i = 0; i < typelib->TypeInfoCount; ++i) + for (UINT i = 0; i < typelib->TypeInfoCount; ++i) { - if (!lstrcmpiW(TLB_get_bstr(typelib->typeinfos[i]->Name), name)) + if (!lstrcmpiW(TLB_get_bstr(typelib->typeinfos[i]->Name), name)) { + if (index) *index = i; return typelib->typeinfos[i]; + } } return NULL; @@ -5482,7 +5482,7 @@ static HRESULT WINAPI ITypeLibComp_fnBindType( if(!szName || !ppTInfo || !ppTComp) return E_INVALIDARG; - info = TLB_get_typeinfo_by_name(This, szName); + info = TLB_get_typeinfo_by_name(This, szName, NULL); if(!info){ *ppTInfo = NULL; *ppTComp = NULL; @@ -8858,7 +8858,7 @@ static HRESULT WINAPI ICreateTypeLib2_fnCreateTypeInfo(ICreateTypeLib2 *iface, if (!ctinfo || !name) return E_INVALIDARG; - info = TLB_get_typeinfo_by_name(This, name); + info = TLB_get_typeinfo_by_name(This, name, NULL); if (info) return TYPE_E_NAMECONFLICT; @@ -10004,10 +10004,9 @@ static void WMSFT_write_segment(HANDLE outfile, WMSFT_SegContents *segment) static HRESULT WMSFT_fixup_typeinfos(ITypeLibImpl *This, WMSFT_TLBFile *file, DWORD file_len) { - DWORD i; MSFT_TypeInfoBase *base = (MSFT_TypeInfoBase *)file->typeinfo_seg.data; - for(i = 0; i < This->TypeInfoCount; ++i){ + for(UINT i = 0; i < This->TypeInfoCount; ++i){ base->memoffset += file_len; ++base; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7203
From: Edward O'Callaghan <edward(a)antitrust.cc> Implements the trivial cases of 'ICreateTypeLib2::DeleteTypeInfo'. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57662 Signed-off-by: Edward O'Callaghan <edward(a)antitrust.cc> --- dlls/oleaut32/typelib.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index 226d2f4b417..e66e667ad97 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -10250,6 +10250,15 @@ static HRESULT WINAPI ICreateTypeLib2_fnDeleteTypeInfo(ICreateTypeLib2 *iface, LPOLESTR name) { ITypeLibImpl *This = impl_from_ICreateTypeLib2(iface); + ITypeInfoImpl *info; + + if (!name) + return E_INVALIDARG; + + info = TLB_get_typeinfo_by_name(This, name, NULL); + if (!info) + return E_INVALIDARG; + FIXME("%p %s - stub\n", This, wine_dbgstr_w(name)); return E_NOTIMPL; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7203
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=151070 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4306: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000008200EA, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
While typelib.c is definitely not the cleanest of them all, what we need here is some tests for DeleteTypeInfo() to confirm your changes. Other changes should be dropped in my opinion. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7203#note_92890
On Mon Jan 27 16:47:20 2025 +0000, Nikolay Sivov wrote:
While typelib.c is definitely not the cleanest of them all, what we need here is some tests for DeleteTypeInfo() to confirm your changes. Other changes should be dropped in my opinion. Agreed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7203#note_92953
On Mon Jan 27 16:47:20 2025 +0000, Huw Davies wrote:
Agreed. Strongly disagree. The current implementation is virtually unintelligible.
dlls/oleaut32: Simplify loop in ITypeLib2_Constructor_MSFT() is suitable change on its own merit. dlls/oleaut32: Add DeleteTypeInfo() base-cases in typelib.c Agree it needs test cases, I have some locally. While not all the changes are needed, however ref counting in this implementation looks very broken, there is variable re-uses that opens the door to use-after-frees and a number of other distasteful things. For how simple the conceptual state machine is, the whole thing is virtually impossible to contribute to for any outsider in it's current form. Therefore some work is needed to lower the bar some. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7203#note_92962
participants (5)
-
Edward O'Callaghan -
Edward OC (＠funfunctor)
-
Huw Davies (＠huw)
-
Marvin -
Nikolay Sivov (＠nsivov)