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

[Michael Stefaniuc]

On 9/15/21 20:39, Dmitry Timoshkov wrote:
> Michael Stefaniuc  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