[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
From: Kevin Martinez 137180189+kevinrmartinez@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); +}
From: Kevin Martinez 137180189+kevinrmartinez@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} };
From: Kevin Martinez 137180189+kevinrmartinez@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();
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.
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.
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.
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?
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.
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.
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.
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.
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.
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.
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.
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".
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.
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).
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).
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!
-
Alfred Agrell (＠Alcaro) -
Elizabeth Figura (＠zfigura)
-
Kevin Martinez
-
Kevin Martinez (＠kevinrmartinez)
-
Marvin