[PATCH] sxs: Don't assign a COM object to the 3rd param of QueryInterface

[Michael Stefaniuc]

Signed-off-by: Michael Stefaniuc <mstefani(a)winehq.org>
---
 dlls/sxs/cache.c | 10 ++++------
 dlls/sxs/name.c  | 10 ++++------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/dlls/sxs/cache.c b/dlls/sxs/cache.c
index 0acf012032d..e4fdda8c860 100644
--- a/dlls/sxs/cache.c
+++ b/dlls/sxs/cache.c
@@ -57,19 +57,17 @@ static inline struct cache *impl_from_IAssemblyCache(IAssemblyCache *iface)
 static HRESULT WINAPI cache_QueryInterface(
     IAssemblyCache *iface,
     REFIID riid,
-    void **obj )
+    void **ret_iface )
 {
-    struct cache *cache = impl_from_IAssemblyCache(iface);
-
-    TRACE("%p, %s, %p\n", cache, debugstr_guid(riid), obj);
+    TRACE("%p, %s, %p\n", iface, debugstr_guid(riid), ret_iface);
 
-    *obj = NULL;
+    *ret_iface = NULL;
 
     if (IsEqualIID(riid, &IID_IUnknown) ||
         IsEqualIID(riid, &IID_IAssemblyCache))
     {
         IAssemblyCache_AddRef( iface );
-        *obj = cache;
+        *ret_iface = iface;
         return S_OK;
     }
 
diff --git a/dlls/sxs/name.c b/dlls/sxs/name.c
index 6b3ae5197ac..0fab45c5018 100644
--- a/dlls/sxs/name.c
+++ b/dlls/sxs/name.c
@@ -56,19 +56,17 @@ static inline struct name *impl_from_IAssemblyName( IAssemblyName *iface )
 static HRESULT WINAPI name_QueryInterface(
     IAssemblyName *iface,
     REFIID riid,
-    void **obj )
+    void **ret_iface )
 {
-    struct name *name = impl_from_IAssemblyName( iface );
-
-    TRACE("%p, %s, %p\n", name, debugstr_guid(riid), obj);
+    TRACE("%p, %s, %p\n", iface, debugstr_guid(riid), ret_iface);
 
-    *obj = NULL;
+    *ret_iface = NULL;
 
     if (IsEqualIID( riid, &IID_IUnknown ) ||
         IsEqualIID( riid, &IID_IAssemblyName ))
     {
         IAssemblyName_AddRef( iface );
-        *obj = name;
+        *ret_iface = iface;
         return S_OK;
     }
 
-- 
2.31.1

[Dmitry Timoshkov]

Michael Stefaniuc <mstefani(a)winehq.org> wrote:
> @@ -57,19 +57,17 @@ static inline struct cache *impl_from_IAssemblyCache(IAssemblyCache *iface)
>  static HRESULT WINAPI cache_QueryInterface(
>      IAssemblyCache *iface,
>      REFIID riid,
> -    void **obj )
> +    void **ret_iface )
>  {
> -    struct cache *cache = impl_from_IAssemblyCache(iface);
> -
> -    TRACE("%p, %s, %p\n", cache, debugstr_guid(riid), obj);
> +    TRACE("%p, %s, %p\n", iface, debugstr_guid(riid), ret_iface);
>
> -    *obj = NULL;
> +    *ret_iface = NULL;
>
> if (IsEqualIID(riid, &IID_IUnknown) ||
>          IsEqualIID(riid, &IID_IAssemblyCache))
>      {
>          IAssemblyCache_AddRef( iface );
> -        *obj = cache;
> +        *ret_iface = iface;
>          return S_OK;
>      }

Is that really necessary to rename obj to ret_iface? That doesn't really
make the change clearer IMO, and makes the patch larger.

-- 
Dmitry.

[Michael Stefaniuc]

On 9/15/21 20:39, Dmitry Timoshkov wrote:
> Michael Stefaniuc <mstefani(a)winehq.org> wrote:
> > @@ -57,19 +57,17 @@ static inline struct cache *impl_from_IAssemblyCache(IAssemblyCache *iface)
> >   static HRESULT WINAPI cache_QueryInterface(
> >       IAssemblyCache *iface,
> >       REFIID riid,
> > -    void **obj )
> > +    void **ret_iface )
> >   {
> > -    struct cache *cache = impl_from_IAssemblyCache(iface);
> > -
> > -    TRACE("%p, %s, %p\n", cache, debugstr_guid(riid), obj);
> > +    TRACE("%p, %s, %p\n", iface, debugstr_guid(riid), ret_iface);
> >
> > -    *obj = NULL;
> > +    *ret_iface = NULL;
> >
> > if (IsEqualIID(riid, &IID_IUnknown) ||
> >           IsEqualIID(riid, &IID_IAssemblyCache))
> >       {
> >           IAssemblyCache_AddRef( iface );
> > -        *obj = cache;
> > +        *ret_iface = iface;
> >           return S_OK;
> >       }
>
> Is that really necessary to rename obj to ret_iface? That doesn't really
> make the change clearer IMO, and makes the patch larger.

*obj = iface;

That looks just wrong to me... but I spend probably too much qwality time with 
COM in Wine.

sxs was one of the earlier COM cleanups so predating some of the scope creep 
that happened during that effort. As part of the scope creep Jacek and Alexandre 
agreed on replacing obj with ret_iface for the 3rd parameter of QI.
As I already touch two lines with "obj" in them I'm applying that change too.

Also this change is trivial to validate as the exact same object files are 
generated when compiling with -g0.

Reference:
Sadly we lost the COM Cleanup wiki page and 
https://wiki.winehq.org/Wine_Developer%27s_Guide/COM_in_Wine is a hodgepodge 
between obj and ret_iface.


bye
	michael