[PATCH v5 0/2] 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 -- v5: shell32: Added stub for IObjectCollection interface. shell32: Added stub for IEnumObjects interface. 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
Alfred Agrell (@Alcaro) commented about dlls/shell32/enumobjects.c:
FIXME("(%p/%p)->(%n): stub!\n", This, iface, count);
- if (count) - count = 0; - - return S_FALSE; + return E_NOTIMPL;
Well, that's one way to remove the if (count) check. Looks good to me, if it works in the games. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77046
On Thu Jul 25 00:03:34 2024 +0000, Kevin Martinez wrote:
Thanks, any review is appreciated. I actually though I'll have to wait days, even weeks for anyone to look at my code, based on the others MR open. Yeah, there's a lot of luck involved, depending on how much else the maintainers have to do (they're forgetful when overworked, which is always), which part you're contributing to (some parts have no assigned maintainer at all), and whether any volunteer reviewer finds it eye-catching.
I think the recommendation is to complain on IRC if you're ignored. Waiting a week won't help; they're forgetful, not slow. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77049
Alfred Agrell (@Alcaro) commented about dlls/shell32/enumobjects.c:
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;
There's tabs here too -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77048
Alfred Agrell (@Alcaro) commented about dlls/shell32/enumobjects.c:
struct enum_objects { - IEnumObjects IEnumObjects_iface; + IEnumObjects IEnumObjects_iface; IObjectCollection IObjectCollection_iface; - LONG ref; + LONG ref;
Yes, that lines up ...if tab width is set to 4. Text editor shenanigans again, I assume? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77047
Fabian Maurer (@DarkShadow44) 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(
Probably should look like ``` [local] HRESULT Next( [in] ULONG celt, [in] REFIID riid, [out, size_is(celt), length_is(*pceltFetched), iid_is(riid)] void **rgelt, [out] ULONG *pceltFetched); ``` I *think* the annotations are optional, but the local stuff might be needed for... something. I don't know, probably best to have it. Not sure where you got that `pceltFetched` [optional] from, I didn't find that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77062
On Thu Jul 25 18:25:31 2024 +0000, Fabian Maurer wrote:
Probably should look like ``` [local] HRESULT Next( [in] ULONG celt, [in] REFIID riid, [out, size_is(celt), length_is(*pceltFetched), iid_is(riid)] void **rgelt, [out] ULONG *pceltFetched); ``` I *think* the annotations are optional, but the local stuff might be needed for... something. I don't know, probably best to have it. Not sure where you got that `pceltFetched` [optional] from, I didn't find that. The annotations mostly affect COM remoting. They're probably also theoretically usable for some forms of bounds checking, but I'm not aware of any implementation thereof.
Yes, that [optional] does indeed not belong. The argument is nullable, but that tag is only for VARIANT and VARIANT* type arguments. Probably related to calling the function from Visual Basic. [local] bans remoting on that function. I'd either skip it and assume the default handler is good enough, or add that tag to half of both IEnumObjects and IObjectCollection (and change it when and if anyone complains). I can't find a clear answer for how [size_is] and [length_is] interact, but I do know that *pceltFetched is not a number if pceltFetched is null. Personally I'd include [size_is] and skip [length_is], unless someone who actually understands this stuff tells me otherwise. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77063
On Thu Jul 25 19:45:13 2024 +0000, Alfred Agrell wrote:
The annotations mostly affect COM remoting. They're probably also theoretically usable for some forms of bounds checking, but I'm not aware of any implementation thereof. Yes, that [optional] does indeed not belong. The argument is nullable, but that tag is only for VARIANT and VARIANT* type arguments. Probably related to calling the function from Visual Basic. [local] bans remoting on that function. I'd either skip it and assume the default handler is good enough, or add that tag to half of both IEnumObjects and IObjectCollection (and change it when and if anyone complains). I can't find a clear answer for how [size_is] and [length_is] interact, but I do know that *pceltFetched is not a number if pceltFetched is null. Personally I'd include [size_is] and skip [length_is], unless someone who actually understands this stuff tells me otherwise. We need the same annotations as native for anything that's remotable, which this is, and especially for anything that's in a native typelib, which this also is. That means that we do indeed need the [local] before Next, as Fabian has it, with the same size_is and length_is on "rgelt", and we also need the RemoteNext method.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77070
On Thu Jul 25 20:42:38 2024 +0000, Elizabeth Figura wrote:
We need the same annotations as native for anything that's remotable, which this is, and especially for anything that's in a native typelib, which this also is. That means that we do indeed need the [local] before Next, as Fabian has it, with the same size_is and length_is on "rgelt", and we also need the RemoteNext method. Okay, you know that stuff better than me. _redacts some pieces from my above post_
So native also has that null-deref length_is clause? I guess that's a bug in native, then. Better do the same, wouldn't be the first blatant bug Wine reproduces. This MR's scope sure is growing... we can at least leave RemoteNext as E_NOTIMPL or something for now, right? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77075
So native also has that null-deref length_is clause? I guess that's a bug in native, then. Better do the same, wouldn't be the first blatant bug Wine reproduces.
That's what RemoteNext is for. RPC doesn't let that be NULL, but the API does. See IEnumShellItems_Next_Proxy() in dlls/actxprxy/usrmarshal.c. This merge request will need to essentially copy-paste that implementation in the same file. RemoteNext() isn't exactly a real method and doesn't need to be implemented per se. It's basically the internal RPC version of Next() that has RPC-friendly parameters. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77086
On Thu Jul 25 21:44:51 2024 +0000, Elizabeth Figura wrote:
So native also has that null-deref length_is clause? I guess that's a bug in native, then. Better do the same, wouldn't be the first blatant bug Wine reproduces. That's what RemoteNext is for. RPC doesn't let that be NULL, but the API does. See IEnumShellItems_Next_Proxy() in dlls/actxprxy/usrmarshal.c. This merge request will need to essentially copy-paste that implementation in the same file. RemoteNext() isn't exactly a real method and doesn't need to be implemented per se. It's basically the internal RPC version of Next() that has RPC-friendly parameters. Ok, let's see if I'm on the same page here. On `include/shobjidl.idl`, aside of the annotations on `Next()`, I should add the RemoteNext method like this? `[call_as(Next)] HRESULT RemoteNext( [in] ULONG celt, [in] REFIID riid, [out, size_is(celt), length_is(*pceltFetched), iid_is(riid)] void **rgelt, [out] ULONG *pceltFetched);`
This merge request will need to essentially copy-paste that implementation in the same file.
So I need to an `IEnumObjects_Next_Proxy()` method based on `IEnumShellItems_Next_Proxy()` on dlls/actxprxy/usrmarshal.c? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77163
On Fri Jul 26 20:16:02 2024 +0000, Kevin Martinez wrote:
Ok, let's see if I'm on the same page here. On `include/shobjidl.idl`, aside of the annotations on `Next()`, I should add the RemoteNext method like this? ``` [call_as(Next)] HRESULT RemoteNext( [in] ULONG celt, [in] REFIID riid, [out, size_is(celt), length_is(*pceltFetched), iid_is(riid)] void **rgelt, [out] ULONG *pceltFetched); ```
This merge request will need to essentially copy-paste that implementation in the same file. So I need to an `IEnumObjects_Next_Proxy()` method based on `IEnumShellItems_Next_Proxy()` on dlls/actxprxy/usrmarshal.c? Yes.
FWIW, I would not put every argument on a separate line. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77165
On Fri Jul 26 21:35:53 2024 +0000, Elizabeth Figura wrote:
Yes. FWIW, I would not put every argument on a separate line. I applied the changes, but the 32bit Wine doesn't compile with those. The whole output of `make` is on the attachment:[error32.txt](/uploads/38cfa818c06ae238f4986b608a4491c7/error32.txt).
The 64bit build does compile however, I just don't know what the error message means. I'm stuck. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77170
On Sat Jul 27 01:34:57 2024 +0000, Kevin Martinez wrote:
I applied the changes, but the 32bit Wine doesn't compile with those. The whole output of `make` is on the attachment:[error32.txt](/uploads/38cfa818c06ae238f4986b608a4491c7/error32.txt). The 64bit build does compile however, I just don't know what the error message means. I'm stuck. Can you please attach the whole diff?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77172
On Sat Jul 27 01:34:57 2024 +0000, Elizabeth Figura wrote:
Can you please attach the whole diff? Sure thing: [RemoteNext_diff.txt](/uploads/5ba25ce2addb791abd189bf249e04894/RemoteNext_diff.txt)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77173
On Sat Jul 27 12:59:42 2024 +0000, Kevin Martinez wrote:
Sure thing: [RemoteNext_diff.txt](/uploads/5ba25ce2addb791abd189bf249e04894/RemoteNext_diff.txt) Attaching minimal sample: [test.idl](/uploads/2b6672159a59789fe0d55bada83800f7/test.idl) This looks like a widl bug. I stumbled into that as well, but I didn't realize it works for 64bit.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77179
On Sat Jul 27 16:52:59 2024 +0000, Fabian Maurer wrote:
Attaching minimal sample: [test.idl](/uploads/2b6672159a59789fe0d55bada83800f7/test.idl) This looks like a widl bug. I stumbled into that as well, but I didn't realize it works for 64bit. FWIW, it diverges at https://gitlab.winehq.org/wine/wine/-/blob/9e43076913d717d9d049b55310c825485... And then it really dislikes the `void**` The basic problem is that we're not properly propagating the iid_is() down to the inner type. It successfully compiles for 64-bit, but generates incorrect type format strings. I'm working on a fix.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77181
On Sat Jul 27 17:29:12 2024 +0000, Elizabeth Figura wrote:
The basic problem is that we're not properly propagating the iid_is() down to the inner type. It successfully compiles for 64-bit, but generates incorrect type format strings. I'm working on a fix. I've sent <https://gitlab.winehq.org/wine/wine/-/merge_requests/6165> for this. The format string still doesn't quite match Windows, apparently because (contradicting the documentation) Windows doesn't seem to treat interface pointers as causing an array to be complex—but it should at least marshal correctly now.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77195
On Sat Jul 27 20:38:51 2024 +0000, Elizabeth Figura wrote:
I've sent <https://gitlab.winehq.org/wine/wine/-/merge_requests/6165> for this. The format string still doesn't quite match Windows, apparently because (contradicting the documentation) Windows doesn't seem to treat interface pointers as causing an array to be complex—but it should at least marshal correctly now. Alright, applying the patch from !6165 now I can compile. So how do I proceed? Should I push my changes now? Or after !6165 gets merged?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6130#note_77252
participants (5)
-
Alfred Agrell (＠Alcaro) -
Elizabeth Figura (＠zfigura)
-
Fabian Maurer (＠DarkShadow44)
-
Kevin Martinez
-
Kevin Martinez (＠kevinrmartinez)