[PATCH v6 0/3] MR6130: shell32: Added stubs for the EnumerableObjectCollection COM class.
Stub added for the IEnumObjects and IObjectCollection interfaces, so that programs won't crash when calling it. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53620 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56891 -- v6: actxprxy: Added IEnumObjects_Next_Proxy https://gitlab.winehq.org/wine/wine/-/merge_requests/6130
From: Kevin Martinez <137180189+kevinrmartinez(a)users.noreply.github.com> --- dlls/shell32/Makefile.in | 1 + dlls/shell32/enumobjects.c | 164 +++++++++++++++++++++++++++++++ dlls/shell32/shell32_classes.idl | 5 + dlls/shell32/shell32_main.h | 1 + dlls/shell32/shellole.c | 1 + include/shobjidl.idl | 18 ++++ 6 files changed, 190 insertions(+) create mode 100644 dlls/shell32/enumobjects.c diff --git a/dlls/shell32/Makefile.in b/dlls/shell32/Makefile.in index 4a906ac59db..c2df32b62ec 100644 --- a/dlls/shell32/Makefile.in +++ b/dlls/shell32/Makefile.in @@ -21,6 +21,7 @@ SOURCES = \ dragdrophelper.c \ ebrowser.c \ enumidlist.c \ + enumobjects.c \ folders.c \ iconcache.c \ new_menu.c \ diff --git a/dlls/shell32/enumobjects.c b/dlls/shell32/enumobjects.c new file mode 100644 index 00000000000..572b76a9cac --- /dev/null +++ b/dlls/shell32/enumobjects.c @@ -0,0 +1,164 @@ +/* + * IEnumObjects + * + * Copyright 2024 Kevin Martinez + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <stdarg.h> +#include <stdlib.h> +#include <string.h> + +#define COBJMACROS + +#include "wine/debug.h" +#include "windef.h" +#include "winbase.h" +#include "winreg.h" +#include "shlwapi.h" + +#include "shell32_main.h" + +WINE_DEFAULT_DEBUG_CHANNEL(shell); + +struct enum_objects +{ + IEnumObjects IEnumObjects_iface; + LONG ref; +}; + +static inline struct enum_objects *impl_from_IEnumObjects(IEnumObjects *iface) +{ + return CONTAINING_RECORD(iface, struct enum_objects, IEnumObjects_iface); +} + +static HRESULT WINAPI enum_objects_QueryInterface(IEnumObjects *iface, REFIID riid, void **obj) +{ + struct enum_objects *This = impl_from_IEnumObjects(iface); + + TRACE("(%p/%p)->(%s, %p)\n", This, iface, debugstr_guid(riid), obj); + + if (IsEqualIID(&IID_IUnknown, riid) || + IsEqualIID(&IID_IEnumObjects, riid)) + { + *obj = &This->IEnumObjects_iface; + } + else + { + WARN("no interface for %s\n", debugstr_guid(riid)); + *obj = NULL; + return E_NOINTERFACE; + } + + IUnknown_AddRef((IUnknown*)*obj); + return S_OK; +} + +static ULONG WINAPI enum_objects_AddRef(IEnumObjects *iface) +{ + struct enum_objects *This = impl_from_IEnumObjects(iface); + ULONG refcount = InterlockedIncrement(&This->ref); + + TRACE("(%p/%p)->(%lu)\n", This, iface, refcount); + + return refcount; +} + + static ULONG WINAPI enum_objects_Release(IEnumObjects *iface) +{ + struct enum_objects *This = impl_from_IEnumObjects(iface); + ULONG refcount = InterlockedDecrement(&This->ref); + + TRACE("(%p/%p)->(%lu)\n", This, iface, refcount); + + if (!refcount) + { + free(This); + } + + return refcount; +} + +static HRESULT WINAPI enum_objects_Next(IEnumObjects *iface, ULONG celt, REFIID riid, void **rgelt, ULONG *celtFetched) +{ + struct enum_objects *This = impl_from_IEnumObjects(iface); + + FIXME("(%p/%p %ld, %p)->(%p, %p): stub!\n", This, iface, celt, debugstr_guid(riid), rgelt, celtFetched); + + if (celtFetched) + celtFetched = 0; + + return S_FALSE; +} + +static HRESULT WINAPI enum_objects_Skip(IEnumObjects *iface, ULONG celt) +{ + struct enum_objects *This = impl_from_IEnumObjects(iface); + + FIXME("(%p/%p %ld): stub!\n", This, iface, celt); + + return E_NOTIMPL; +} + +static HRESULT WINAPI enum_objects_Reset(IEnumObjects *iface) +{ + struct enum_objects *This = impl_from_IEnumObjects(iface); + + FIXME("(%p/%p): stub!\n", This, iface); + + return E_NOTIMPL; +} + +static HRESULT WINAPI enum_objects_Clone(IEnumObjects *iface, IEnumObjects **ppenum) +{ + struct enum_objects *This = impl_from_IEnumObjects(iface); + + FIXME("(%p/%p)->(%p): stub!\n",This, iface, ppenum); + + return E_NOTIMPL; +} + +static const IEnumObjectsVtbl enum_objects_vtbl = +{ + enum_objects_QueryInterface, + enum_objects_AddRef, + enum_objects_Release, + enum_objects_Next, + enum_objects_Skip, + enum_objects_Reset, + enum_objects_Clone, +}; + +HRESULT WINAPI IEnumObjects_Constructor(IUnknown *outer, REFIID riid, void **obj) +{ + struct enum_objects *This; + HRESULT hr; + + TRACE("(%p, %s, %p)\n", outer, debugstr_guid(riid), obj); + + if (outer) + return CLASS_E_NOAGGREGATION; + + if (!(This = heap_alloc(sizeof(*This)))) + return E_OUTOFMEMORY; + + This->ref = 1; + This->IEnumObjects_iface.lpVtbl = &enum_objects_vtbl; + + hr = IEnumObjects_QueryInterface(&This->IEnumObjects_iface, riid, obj); + IEnumObjects_Release(&This->IEnumObjects_iface); + return hr; +} diff --git a/dlls/shell32/shell32_classes.idl b/dlls/shell32/shell32_classes.idl index 135ef52f7b0..872ecb31630 100644 --- a/dlls/shell32/shell32_classes.idl +++ b/dlls/shell32/shell32_classes.idl @@ -189,3 +189,8 @@ coclass KnownFolderManager { interface IKnownFolderManager; } uuid(d969a300-e7ff-11d0-a93b-00a0c90f2719) ] coclass NewMenu {} + +[ + threading(apartment), + uuid(2d3468c1-36a7-43b6-ac24-d3f02fd9607a) +] coclass EnumerableObjectCollection { interface IEnumObjects; } diff --git a/dlls/shell32/shell32_main.h b/dlls/shell32/shell32_main.h index 6b5575c6879..b25cba56317 100644 --- a/dlls/shell32/shell32_main.h +++ b/dlls/shell32/shell32_main.h @@ -103,6 +103,7 @@ HRESULT WINAPI ExplorerBrowser_Constructor(IUnknown *pUnkOuter, REFIID riid, LPV HRESULT WINAPI KnownFolderManager_Constructor(IUnknown *pUnkOuter, REFIID riid, LPVOID *ppv); HRESULT WINAPI IFileOperation_Constructor(IUnknown *outer, REFIID riid, void **out); HRESULT WINAPI ActiveDesktop_Constructor(IUnknown *outer, REFIID riid, void **out); +HRESULT WINAPI IEnumObjects_Constructor(IUnknown *outer, REFIID riid, void **obj); extern HRESULT CPanel_GetIconLocationW(LPCITEMIDLIST, LPWSTR, UINT, int*); HRESULT WINAPI CPanel_ExtractIconA(LPITEMIDLIST pidl, LPCSTR pszFile, UINT nIconIndex, HICON *phiconLarge, HICON *phiconSmall, UINT nIconSize); diff --git a/dlls/shell32/shellole.c b/dlls/shell32/shellole.c index aa9bd3e0f3e..b9cf6e633a0 100644 --- a/dlls/shell32/shellole.c +++ b/dlls/shell32/shellole.c @@ -87,6 +87,7 @@ static const struct { {&CLSID_ShellImageDataFactory, ShellImageDataFactory_Constructor}, {&CLSID_FileOperation, IFileOperation_Constructor}, {&CLSID_ActiveDesktop, ActiveDesktop_Constructor}, + {&CLSID_EnumerableObjectCollection, IEnumObjects_Constructor}, {NULL, NULL} }; diff --git a/include/shobjidl.idl b/include/shobjidl.idl index 886cff89ef9..bb87fc31587 100644 --- a/include/shobjidl.idl +++ b/include/shobjidl.idl @@ -4126,3 +4126,21 @@ interface IFileOperation : IUnknown HRESULT PerformOperations(); HRESULT GetAnyOperationsAborted([out] BOOL *aborted); } + +[ + object, + uuid(2c1c7e2e-2d0e-4059-831e-1e6f82335c2e), + pointer_default(unique) +] +interface IEnumObjects : IUnknown +{ + HRESULT Next( + [in] ULONG celt, + [in] REFIID riid, + [out, iid_is(riid)] void **rgelt, + [out, optional] ULONG *pceltFetched ); + + HRESULT Skip([in] ULONG celt); + HRESULT Reset(); + HRESULT Clone([out] IEnumObjects **ppenum); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6130
From: Kevin Martinez <137180189+kevinrmartinez(a)users.noreply.github.com> --- dlls/shell32/enumobjects.c | 132 +++++++++++++++++++++++++++++++----- dlls/shell32/shell32_main.h | 2 +- dlls/shell32/shellole.c | 2 +- 3 files changed, 117 insertions(+), 19 deletions(-) diff --git a/dlls/shell32/enumobjects.c b/dlls/shell32/enumobjects.c index 572b76a9cac..2b769f49f68 100644 --- a/dlls/shell32/enumobjects.c +++ b/dlls/shell32/enumobjects.c @@ -1,5 +1,5 @@ /* - * IEnumObjects + * EnumerableObjectCollection * * Copyright 2024 Kevin Martinez * @@ -36,8 +36,9 @@ WINE_DEFAULT_DEBUG_CHANNEL(shell); struct enum_objects { - IEnumObjects IEnumObjects_iface; - LONG ref; + IEnumObjects IEnumObjects_iface; + IObjectCollection IObjectCollection_iface; + LONG ref; }; static inline struct enum_objects *impl_from_IEnumObjects(IEnumObjects *iface) @@ -45,26 +46,37 @@ static inline struct enum_objects *impl_from_IEnumObjects(IEnumObjects *iface) return CONTAINING_RECORD(iface, struct enum_objects, IEnumObjects_iface); } +static inline struct enum_objects *impl_from_IObjectCollection(IObjectCollection *iface) +{ + return CONTAINING_RECORD(iface, struct enum_objects, IObjectCollection_iface); +} + static HRESULT WINAPI enum_objects_QueryInterface(IEnumObjects *iface, REFIID riid, void **obj) { struct enum_objects *This = impl_from_IEnumObjects(iface); TRACE("(%p/%p)->(%s, %p)\n", This, iface, debugstr_guid(riid), obj); - if (IsEqualIID(&IID_IUnknown, riid) || - IsEqualIID(&IID_IEnumObjects, riid)) + *obj = NULL; + + if (IsEqualIID(riid, &IID_IUnknown) || IsEqualIID(riid, &IID_IEnumObjects)) { *obj = &This->IEnumObjects_iface; } - else + else if (IsEqualIID(riid, &IID_IObjectCollection) || IsEqualIID(riid, &IID_IObjectArray)) { - WARN("no interface for %s\n", debugstr_guid(riid)); - *obj = NULL; - return E_NOINTERFACE; + *obj = &This->IObjectCollection_iface; } - IUnknown_AddRef((IUnknown*)*obj); - return S_OK; + if (*obj) + { + IUnknown_AddRef((IUnknown*)*obj); + return S_OK; + } + + FIXME("No interface for %s\n", debugstr_guid(riid)); + + return E_NOINTERFACE; } static ULONG WINAPI enum_objects_AddRef(IEnumObjects *iface) @@ -98,17 +110,17 @@ static HRESULT WINAPI enum_objects_Next(IEnumObjects *iface, ULONG celt, REFIID FIXME("(%p/%p %ld, %p)->(%p, %p): stub!\n", This, iface, celt, debugstr_guid(riid), rgelt, celtFetched); - if (celtFetched) - celtFetched = 0; + if (celtFetched) + *celtFetched = 0; - return S_FALSE; + return S_FALSE; } static HRESULT WINAPI enum_objects_Skip(IEnumObjects *iface, ULONG celt) { struct enum_objects *This = impl_from_IEnumObjects(iface); - FIXME("(%p/%p %ld): stub!\n", This, iface, celt); + FIXME("(%p/%p %ld): stub!\n", This, iface, celt); return E_NOTIMPL; } @@ -118,7 +130,7 @@ static HRESULT WINAPI enum_objects_Reset(IEnumObjects *iface) struct enum_objects *This = impl_from_IEnumObjects(iface); FIXME("(%p/%p): stub!\n", This, iface); - + return E_NOTIMPL; } @@ -142,7 +154,92 @@ static const IEnumObjectsVtbl enum_objects_vtbl = enum_objects_Clone, }; -HRESULT WINAPI IEnumObjects_Constructor(IUnknown *outer, REFIID riid, void **obj) +static HRESULT WINAPI object_collection_QueryInterface(IObjectCollection *iface, REFIID riid, void **obj) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + return IEnumObjects_QueryInterface(&This->IEnumObjects_iface, riid, obj); +} + +static ULONG WINAPI object_collection_AddRef(IObjectCollection *iface) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + return IEnumObjects_AddRef(&This->IEnumObjects_iface); +} + +static ULONG WINAPI object_collection_Release(IObjectCollection *iface) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + return IEnumObjects_Release(&This->IEnumObjects_iface); +} + +static HRESULT WINAPI object_collection_GetCount(IObjectCollection *iface, UINT *count) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p)->(%n): stub!\n", This, iface, count); + + return E_NOTIMPL; +} + +static HRESULT WINAPI object_collection_GetAt(IObjectCollection *iface, UINT index, REFIID riid, void **obj) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p %d, %s)->(%p): stub!\n", This, iface, index, debugstr_guid(riid), obj); + + return E_NOTIMPL; +} + +static HRESULT WINAPI object_collection_AddObject(IObjectCollection *iface, IUnknown *obj) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p %p): stub!\n", This, iface, obj); + + return E_NOTIMPL; +} + +static HRESULT WINAPI object_collection_AddFromArray(IObjectCollection *iface, IObjectArray *source_array) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p %p): stub!\n", This, iface, source_array); + + return E_NOTIMPL; +} + +static HRESULT WINAPI object_collection_RemoveObjectAt(IObjectCollection *iface, UINT index) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p %i): stub!\n", This, iface, index); + + return E_NOTIMPL; +} + +static HRESULT WINAPI object_collection_Clear(IObjectCollection *iface) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p): stub!\n", This, iface); + + return E_NOTIMPL; +} + +static const IObjectCollectionVtbl object_collection_vtbl = +{ + object_collection_QueryInterface, + object_collection_AddRef, + object_collection_Release, + object_collection_GetCount, + object_collection_GetAt, + object_collection_AddObject, + object_collection_AddFromArray, + object_collection_RemoveObjectAt, + object_collection_Clear +}; + +HRESULT WINAPI EnumerableObjectCollection_Constructor(IUnknown *outer, REFIID riid, void **obj) { struct enum_objects *This; HRESULT hr; @@ -157,6 +254,7 @@ HRESULT WINAPI IEnumObjects_Constructor(IUnknown *outer, REFIID riid, void **obj This->ref = 1; This->IEnumObjects_iface.lpVtbl = &enum_objects_vtbl; + This->IObjectCollection_iface.lpVtbl = &object_collection_vtbl; hr = IEnumObjects_QueryInterface(&This->IEnumObjects_iface, riid, obj); IEnumObjects_Release(&This->IEnumObjects_iface); diff --git a/dlls/shell32/shell32_main.h b/dlls/shell32/shell32_main.h index b25cba56317..c4b301f500a 100644 --- a/dlls/shell32/shell32_main.h +++ b/dlls/shell32/shell32_main.h @@ -103,7 +103,7 @@ HRESULT WINAPI ExplorerBrowser_Constructor(IUnknown *pUnkOuter, REFIID riid, LPV HRESULT WINAPI KnownFolderManager_Constructor(IUnknown *pUnkOuter, REFIID riid, LPVOID *ppv); HRESULT WINAPI IFileOperation_Constructor(IUnknown *outer, REFIID riid, void **out); HRESULT WINAPI ActiveDesktop_Constructor(IUnknown *outer, REFIID riid, void **out); -HRESULT WINAPI IEnumObjects_Constructor(IUnknown *outer, REFIID riid, void **obj); +HRESULT WINAPI EnumerableObjectCollection_Constructor(IUnknown *outer, REFIID riid, void **obj); extern HRESULT CPanel_GetIconLocationW(LPCITEMIDLIST, LPWSTR, UINT, int*); HRESULT WINAPI CPanel_ExtractIconA(LPITEMIDLIST pidl, LPCSTR pszFile, UINT nIconIndex, HICON *phiconLarge, HICON *phiconSmall, UINT nIconSize); diff --git a/dlls/shell32/shellole.c b/dlls/shell32/shellole.c index b9cf6e633a0..fc3f6b032cd 100644 --- a/dlls/shell32/shellole.c +++ b/dlls/shell32/shellole.c @@ -87,7 +87,7 @@ static const struct { {&CLSID_ShellImageDataFactory, ShellImageDataFactory_Constructor}, {&CLSID_FileOperation, IFileOperation_Constructor}, {&CLSID_ActiveDesktop, ActiveDesktop_Constructor}, - {&CLSID_EnumerableObjectCollection, IEnumObjects_Constructor}, + {&CLSID_EnumerableObjectCollection, EnumerableObjectCollection_Constructor}, {NULL, NULL} }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6130
From: Kevin Martinez <137180189+kevinrmartinez(a)users.noreply.github.com> --- dlls/actxprxy/usrmarshal.c | 18 ++++++++++++++++++ dlls/shell32/enumobjects.c | 12 ++++++------ include/shobjidl.idl | 14 +++++++++++--- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/dlls/actxprxy/usrmarshal.c b/dlls/actxprxy/usrmarshal.c index e4eada6848f..5fa3c8581ec 100644 --- a/dlls/actxprxy/usrmarshal.c +++ b/dlls/actxprxy/usrmarshal.c @@ -247,3 +247,21 @@ HRESULT __RPC_STUB IParentAndItem_GetParentAndItem_Proxy( TRACE("(%p)->(%p %p %p)\n", This, parent, folder, child); return IParentAndItem_RemoteGetParentAndItem_Proxy(This, parent, folder, child); } + +HRESULT CALLBACK IEnumObjects_Next_Proxy(IEnumObjects *This, ULONG celt, REFIID riid, void **rgelt, ULONG *pceltFetched) +{ + ULONG fetched; + TRACE("(%p)->(%ld, %p, %p, %p)\n", This, celt, debugstr_guid(riid), rgelt, pceltFetched); + if (!pceltFetched) pceltFetched = &fetched; + return IEnumObjects_RemoteNext_Proxy(This, celt, riid, rgelt, pceltFetched); +} + +HRESULT __RPC_STUB IEnumObjects_Next_Stub(IEnumObjects *This, ULONG celt, REFIID riid, void **rgelt, ULONG *pceltFetched) +{ + HRESULT hr; + TRACE("(%p)->(%ld, %p, %p, %p)\n", This, celt, debugstr_guid(riid), rgelt, pceltFetched); + *pceltFetched = 0; + hr = IEnumObjects_Next(This, celt, riid, rgelt, pceltFetched); + if (hr == S_OK) *pceltFetched = celt; + return hr; +} diff --git a/dlls/shell32/enumobjects.c b/dlls/shell32/enumobjects.c index 2b769f49f68..770f995b9d1 100644 --- a/dlls/shell32/enumobjects.c +++ b/dlls/shell32/enumobjects.c @@ -36,9 +36,9 @@ WINE_DEFAULT_DEBUG_CHANNEL(shell); struct enum_objects { - IEnumObjects IEnumObjects_iface; + IEnumObjects IEnumObjects_iface; IObjectCollection IObjectCollection_iface; - LONG ref; + LONG ref; }; static inline struct enum_objects *impl_from_IEnumObjects(IEnumObjects *iface) @@ -109,11 +109,11 @@ static HRESULT WINAPI enum_objects_Next(IEnumObjects *iface, ULONG celt, REFIID struct enum_objects *This = impl_from_IEnumObjects(iface); FIXME("(%p/%p %ld, %p)->(%p, %p): stub!\n", This, iface, celt, debugstr_guid(riid), rgelt, celtFetched); - - if (celtFetched) + + if (celtFetched) *celtFetched = 0; - - return S_FALSE; + + return S_FALSE; } static HRESULT WINAPI enum_objects_Skip(IEnumObjects *iface, ULONG celt) diff --git a/include/shobjidl.idl b/include/shobjidl.idl index bb87fc31587..624545b80db 100644 --- a/include/shobjidl.idl +++ b/include/shobjidl.idl @@ -4134,11 +4134,19 @@ interface IFileOperation : IUnknown ] interface IEnumObjects : IUnknown { - HRESULT Next( + [local] HRESULT Next( + [in] ULONG celt, + [in] REFIID riid, + [out, size_is(celt), length_is(*pceltFetched), iid_is(riid)] void **rgelt, + [out] ULONG *pceltFetched + ); + + [call_as(Next)] HRESULT RemoteNext( [in] ULONG celt, [in] REFIID riid, - [out, iid_is(riid)] void **rgelt, - [out, optional] ULONG *pceltFetched ); + [out, size_is(celt), length_is(*pceltFetched), iid_is(riid)] void **rgelt, + [out] ULONG *pceltFetched + ); HRESULT Skip([in] ULONG celt); HRESULT Reset(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6130
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147680 Your paranoid android. === debian11 (build log) === Task: The win32 Wine build failed === debian11b (build log) === Task: The wow32 Wine build failed
On Sun Aug 11 17:31:34 2024 +0000, Kevin Martinez wrote:
changed this line in [version 6 of the diff](/wine/wine/-/merge_requests/6130/diffs?diff_id=126013&start_sha=147e7a327f3a837b05a832dd78d0d0569a158b90#9718da1edbba0f34e8a2e9e282c5e291ab926c91_41_41) Yeah, but I figured what was wrong.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78379
Another commit added to add code on actxprxy to address [this thread](https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77062). Due to a bug spotted by DarkShadow44 and zfigura in widl, this cannot be merged before !6165. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78381
Elizabeth Figura (@zfigura) commented about dlls/shell32/shell32_main.h:
HRESULT WINAPI KnownFolderManager_Constructor(IUnknown *pUnkOuter, REFIID riid, LPVOID *ppv); HRESULT WINAPI IFileOperation_Constructor(IUnknown *outer, REFIID riid, void **out); HRESULT WINAPI ActiveDesktop_Constructor(IUnknown *outer, REFIID riid, void **out); -HRESULT WINAPI IEnumObjects_Constructor(IUnknown *outer, REFIID riid, void **obj); +HRESULT WINAPI EnumerableObjectCollection_Constructor(IUnknown *outer, REFIID riid, void **obj); Instead of introducing a name in patch 1 and changing it in patch 2, let's just apply this hunk to the first patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78490
Elizabeth Figura (@zfigura) commented about dlls/shell32/enumobjects.c:
+} + +static HRESULT WINAPI object_collection_AddFromArray(IObjectCollection *iface, IObjectArray *source_array) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p %p): stub!\n", This, iface, source_array); + + return E_NOTIMPL; +} + +static HRESULT WINAPI object_collection_RemoveObjectAt(IObjectCollection *iface, UINT index) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p %i): stub!\n", This, iface, index); Unsigned values use %u. Also, why trace both the object and interface pointer?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78491
Elizabeth Figura (@zfigura) commented about dlls/shell32/enumobjects.c:
struct enum_objects { - IEnumObjects IEnumObjects_iface; - LONG ref; + IEnumObjects IEnumObjects_iface; + IObjectCollection IObjectCollection_iface; + LONG ref;
Let's avoid tabs in new code, please. Also, generally we don't align field names like that anymore. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78494
Elizabeth Figura (@zfigura) commented about dlls/shell32/enumobjects.c:
- *obj = NULL; - return E_NOINTERFACE; + *obj = &This->IObjectCollection_iface; }
- IUnknown_AddRef((IUnknown*)*obj); - return S_OK; + if (*obj) + { + IUnknown_AddRef((IUnknown*)*obj); + return S_OK; + } + + FIXME("No interface for %s\n", debugstr_guid(riid)); + + return E_NOINTERFACE; This and other refactorings and whitespace changes should be applied to the patch where the code is introduced.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78493
Elizabeth Figura (@zfigura) commented about dlls/shell32/enumobjects.c:
+{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + return IEnumObjects_AddRef(&This->IEnumObjects_iface); +} + +static ULONG WINAPI object_collection_Release(IObjectCollection *iface) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + return IEnumObjects_Release(&This->IEnumObjects_iface); +} + +static HRESULT WINAPI object_collection_GetCount(IObjectCollection *iface, UINT *count) +{ + struct enum_objects *This = impl_from_IObjectCollection(iface); + + FIXME("(%p/%p)->(%n): stub!\n", This, iface, count); That's not what %n is for.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78492
Elizabeth Figura (@zfigura) commented about dlls/shell32/shell32_classes.idl:
uuid(d969a300-e7ff-11d0-a93b-00a0c90f2719) ] coclass NewMenu {} + +[ + threading(apartment), + uuid(2d3468c1-36a7-43b6-ac24-d3f02fd9607a) +] coclass EnumerableObjectCollection { interface IEnumObjects; }
The "interface" here doesn't actually do anything. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78495
Elizabeth Figura (@zfigura) commented about include/shobjidl.idl:
HRESULT PerformOperations(); HRESULT GetAnyOperationsAborted([out] BOOL *aborted); } + +[ + object, + uuid(2c1c7e2e-2d0e-4059-831e-1e6f82335c2e), + pointer_default(unique) +] +interface IEnumObjects : IUnknown +{ + HRESULT Next( + [in] ULONG celt, + [in] REFIID riid, + [out, iid_is(riid)] void **rgelt, + [out, optional] ULONG *pceltFetched );
I don't think there's any reason to put every argument on a separate line. Also, the spacing is inconsistent. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78496
The hunks in 3/3 need to be fixed up to 1/3 or 2/3 as appropriate. Also, what version of Windows are you seeing vend this class from shell32? My Windows 10 vends it from windows.storage.dll. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78497
On Wed Jul 24 21:29:40 2024 +0000, Kevin Martinez wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/6130/diffs?diff_id=123641&start_sha=3a742be0abca13294046d4e8af5d95534bb551fe#9718da1edbba0f34e8a2e9e282c5e291ab926c91_61_68) WARN is often preferred because querying for unsupported interfaces is often common.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78498
On Wed Jul 24 21:17:49 2024 +0000, Kevin Martinez wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/6130/diffs?diff_id=123639&start_sha=2fbe5633507f80116671c4aed0d834de4369f9ff#9718da1edbba0f34e8a2e9e282c5e291ab926c91_87_77) This is why I prefer the traces used in d3d, which explicitly makes it clear—i.e. "increasing refcount to %u" / "decreasing refcount to %u".
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78499
On Mon Aug 12 22:16:09 2024 +0000, Kevin Martinez wrote:
My editor again... I think using a single space is pretty much universal, at least in new code. But as always consistency is most important.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78500
The hunks in 3/3 need to be fixed up to 1/3 or 2/3 as appropriate.
Sorry but I don't know what that means. You mean like fix the code down to 2 or 1 commit?
what version of Windows are you seeing vend this class from shell32?
To be honest, I didn't personally check what library provided this class, I just followed some hints, especially [from this bug](https://bugs.winehq.org/show_bug.cgi?id=53620). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78542
On Tue Aug 13 14:32:40 2024 +0000, Kevin Martinez wrote:
The hunks in 3/3 need to be fixed up to 1/3 or 2/3 as appropriate. Sorry but I don't know what that means. You mean like fix the code down to 2 or 1 commit? what version of Windows are you seeing vend this class from shell32? To be honest, I didn't personally check what library provided this class, I just followed some hints, especially [from this bug](https://bugs.winehq.org/show_bug.cgi?id=53620). hunk = part of a git commit. In this case, the changes to enumobjects.c should be moved from commit 3 to commit 2, and then the changes to enum_objects_Next/Skip/Reset should be moved to commit 1.
This won't change the final state of the git tree, but [the Wine project values](https://wiki.winehq.org/Submitting_Patches#Managing_your_submission) each commit being clean and not introducing style issues that get fixed in next commit. There are many ways to shuffle around commits, but one method is [splitting the commits](https://stackoverflow.com/questions/6217156/break-a-previous-commit-into-mul...), then git rebase -i to merge and move the pieces where they belong (swap lines, and change pick to fixup). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78546
On Tue Aug 13 15:33:56 2024 +0000, Alfred Agrell wrote:
hunk = part of a git commit. In this case, the changes to enumobjects.c should be moved from commit 3 to commit 2, and then the changes to enum_objects_Next/Skip/Reset should be moved to commit 1. This won't change the final state of the git tree, but [the Wine project values](https://wiki.winehq.org/Submitting_Patches#Managing_your_submission) each commit being clean and not introducing style issues that get fixed in next commit. There are many ways to shuffle around commits, but one method is [splitting the commits](https://stackoverflow.com/questions/6217156/break-a-previous-commit-into-mul...), then git rebase -i to merge and move the pieces where they belong (swap lines, and change pick to fixup). Ok, I get it now, keeping the commit clean by themselves, fixing mistakes were the code was introduced, and not in later ones. Thanks!
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_78566
participants (5)
-
Alfred Agrell (＠Alcaro) -
Elizabeth Figura (＠zfigura)
-
Kevin Martinez
-
Kevin Martinez (＠kevinrmartinez)
-
Marvin