[PATCH v4 0/3] MR7375: dlls/oleaut32: Consistently use TLB_REF_NOT_FOUND

[Edward OC (＠funfunctor)]

Trivial, small set of cleanups patches to start with, non-controversial and NOP functionally speaking.

These are to help get the review process going in making ways towards the bar being lowered for contributing to the oleaut32 implementation.

Signed-off-by: Edward O'Callaghan

--
  v4: dlls/oleaut32: Invert if-stmt in ITypeLib2_fnRelease()
      dlls/oleaut32: Invert if stmt for clarity
      dlls/oleaut32: Consistently use TLB_REF_NOT_FOUND

https://gitlab.winehq.org/wine/wine/-/merge_requests/7375

[PATCH v4 1/3] dlls/oleaut32: Consistently use TLB_REF_NOT_FOUND

[Edward O'Callaghan]

From: Edward O'Callaghan

Signed-off-by: Edward O'Callaghan 
---
 dlls/oleaut32/typelib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c
index c4f00ca5bea..7a57fad58f9 100644
--- a/dlls/oleaut32/typelib.c
+++ b/dlls/oleaut32/typelib.c
@@ -1448,7 +1448,7 @@ static void dump_TLBFuncDescOne(const TLBFuncDesc * pfd)
   MESSAGE("\thelpstring: %s\n", debugstr_w(TLB_get_bstr(pfd->HelpString)));
   if(pfd->Entry == NULL)
       MESSAGE("\tentry: (null)\n");
-  else if(pfd->Entry == (void*)-1)
+  else if(pfd->Entry == TLB_REF_NOT_FOUND)
       MESSAGE("\tentry: invalid\n");
   else if(IS_INTRESOURCE(pfd->Entry))
       MESSAGE("\tentry: %p\n", pfd->Entry);
@@ -7764,7 +7764,7 @@ static HRESULT WINAPI ITypeInfo_fnGetDllEntry( ITypeInfo2 *iface, MEMBERID memid

if (pBstrDllName) *pBstrDllName = SysAllocString(TLB_get_bstr(This->DllName));

-    if (!IS_INTRESOURCE(pFDesc->Entry) && (pFDesc->Entry != (void*)-1))
+    if (!IS_INTRESOURCE(pFDesc->Entry) && (pFDesc->Entry != TLB_REF_NOT_FOUND))
     {
         if (pBstrName) *pBstrName = SysAllocString(TLB_get_bstr(pFDesc->Entry));
         if (pwOrdinal) *pwOrdinal = -1;

[PATCH v4 2/3] dlls/oleaut32: Invert if stmt for clarity

[Edward O'Callaghan]

From: Edward O'Callaghan

Signed-off-by: Edward O'Callaghan 
---
 dlls/oleaut32/typelib.c | 96 +++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 47 deletions(-)

diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c
index 7a57fad58f9..4923bf9a7c4 100644
--- a/dlls/oleaut32/typelib.c
+++ b/dlls/oleaut32/typelib.c
@@ -2020,17 +2020,17 @@ static HRESULT TLB_size_instance(ITypeInfoImpl *info, SYSKIND sys,

static inline void MSFT_Seek(TLBContext *pcx, LONG where)
 {
-    if (where != DO_NOT_SEEK)
+    if (where == DO_NOT_SEEK)
+        return;
+
+    where += pcx->oStart;
+    if (where > pcx->length)
     {
-        where += pcx->oStart;
-        if (where > pcx->length)
-        {
-            /* FIXME */
-            ERR("seek beyond end (%ld/%d)\n", where, pcx->length );
-            TLB_abort();
-        }
-        pcx->pos = where;
+        /* FIXME */
+        ERR("seek beyond end (%ld/%d)\n", where, pcx->length );
+        TLB_abort();
     }
+    pcx->pos = where;
 }

/* read function */
@@ -3215,16 +3215,17 @@ static ULONG WINAPI TLB_Mapping_Release(IUnknown *iface)
 {
     TLB_Mapping *This = mapping_impl_from_IUnknown(iface);
     ULONG refs = InterlockedDecrement(&This->refs);
-    if (!refs)
-    {
-        if (This->typelib_base)
-            UnmapViewOfFile(This->typelib_base);
-        if (This->mapping)
-            CloseHandle(This->mapping);
-        if (This->file != INVALID_HANDLE_VALUE)
-            CloseHandle(This->file);
-        free(This);
-    }
+    if (refs)
+        return refs;
+
+    if (This->typelib_base)
+        UnmapViewOfFile(This->typelib_base);
+    if (This->mapping)
+        CloseHandle(This->mapping);
+    if (This->file != INVALID_HANDLE_VALUE)
+        CloseHandle(This->file);
+    free(This);
+
     return refs;
 }

@@ -5589,21 +5590,20 @@ static const ITypeCompVtbl tlbtcvt =
 /*================== ITypeInfo(2) Methods ===================================*/
 static ITypeInfoImpl* ITypeInfoImpl_Constructor(void)
 {
-    ITypeInfoImpl *pTypeInfoImpl;
+    ITypeInfoImpl *pTypeInfoImpl = calloc(1, sizeof(ITypeInfoImpl));
+    if (!pTypeInfoImpl)
+        return NULL;
+
+    pTypeInfoImpl->ITypeInfo2_iface.lpVtbl = &tinfvt;
+    pTypeInfoImpl->ITypeComp_iface.lpVtbl = &tcompvt;
+    pTypeInfoImpl->ICreateTypeInfo2_iface.lpVtbl = &CreateTypeInfo2Vtbl;
+    pTypeInfoImpl->ref = 0;
+    pTypeInfoImpl->hreftype = -1;
+    pTypeInfoImpl->typeattr.memidConstructor = MEMBERID_NIL;
+    pTypeInfoImpl->typeattr.memidDestructor = MEMBERID_NIL;
+    pTypeInfoImpl->pcustdata_list = &pTypeInfoImpl->custdata_list;
+    list_init(pTypeInfoImpl->pcustdata_list);

-    pTypeInfoImpl = calloc(1, sizeof(ITypeInfoImpl));
-    if (pTypeInfoImpl)
-    {
-      pTypeInfoImpl->ITypeInfo2_iface.lpVtbl = &tinfvt;
-      pTypeInfoImpl->ITypeComp_iface.lpVtbl = &tcompvt;
-      pTypeInfoImpl->ICreateTypeInfo2_iface.lpVtbl = &CreateTypeInfo2Vtbl;
-      pTypeInfoImpl->ref = 0;
-      pTypeInfoImpl->hreftype = -1;
-      pTypeInfoImpl->typeattr.memidConstructor = MEMBERID_NIL;
-      pTypeInfoImpl->typeattr.memidDestructor = MEMBERID_NIL;
-      pTypeInfoImpl->pcustdata_list = &pTypeInfoImpl->custdata_list;
-      list_init(pTypeInfoImpl->pcustdata_list);
-    }
     TRACE("(%p)\n", pTypeInfoImpl);
     return pTypeInfoImpl;
 }
@@ -5630,13 +5630,15 @@ static HRESULT WINAPI ITypeInfo_fnQueryInterface(
     else if(IsEqualIID(riid, &IID_ITypeComp))
         *ppvObject = &This->ITypeComp_iface;

-    if(*ppvObject){
-        IUnknown_AddRef((IUnknown*)*ppvObject);
-        TRACE("-- Interface: (%p)->(%p)\n",ppvObject,*ppvObject);
-        return S_OK;
+    if(!*ppvObject){
+        TRACE("-- Interface: E_NOINTERFACE\n");
+        return E_NOINTERFACE;
     }
-    TRACE("-- Interface: E_NOINTERFACE\n");
-    return E_NOINTERFACE;
+
+    IUnknown_AddRef((IUnknown*)*ppvObject);
+    TRACE("-- Interface: (%p)->(%p)\n",ppvObject,*ppvObject);
+
+    return S_OK;
 }

static ULONG WINAPI ITypeInfo_fnAddRef( ITypeInfo2 *iface)
@@ -5710,17 +5712,17 @@ static ULONG WINAPI ITypeInfo_fnRelease(ITypeInfo2 *iface)
 {
     ITypeInfoImpl *This = impl_from_ITypeInfo2(iface);
     ULONG ref = InterlockedDecrement(&This->ref);
+    BOOL not_attached_to_typelib = This->not_attached_to_typelib;

TRACE("%p, refcount %lu.\n", iface, ref);

-    if (!ref)
-    {
-        BOOL not_attached_to_typelib = This->not_attached_to_typelib;
-        ITypeLib2_Release(&This->pTypeLib->ITypeLib2_iface);
-        if (not_attached_to_typelib)
-            free(This);
-        /* otherwise This will be freed when typelib is freed */
-    }
+    if (ref)
+        return ref;
+
+    ITypeLib2_Release(&This->pTypeLib->ITypeLib2_iface);
+    if (not_attached_to_typelib)
+        free(This);
+    /* otherwise This will be freed when typelib is freed */

return ref;
 }

[PATCH v4 3/3] dlls/oleaut32: Invert if-stmt in ITypeLib2_fnRelease()

[Edward O'Callaghan]

From: Edward O'Callaghan

Handle base condition first, returning as required or else
follow normal procedural flow.

Signed-off-by: Edward O'Callaghan 
---
 dlls/oleaut32/typelib.c | 109 ++++++++++++++++++++--------------------
 1 file changed, 54 insertions(+), 55 deletions(-)

diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c
index 4923bf9a7c4..be162f80598 100644
--- a/dlls/oleaut32/typelib.c
+++ b/dlls/oleaut32/typelib.c
@@ -4785,6 +4785,10 @@ static ULONG WINAPI ITypeLib2_fnAddRef( ITypeLib2 *iface)
 static ULONG WINAPI ITypeLib2_fnRelease( ITypeLib2 *iface)
 {
     ITypeLibImpl *This = impl_from_ITypeLib2(iface);
+    TLBImpLib *pImpLib, *pImpLibNext;
+    TLBRefType *ref_type, *ref_type_next;
+    TLBString *tlbstr, *tlbstr_next;
+    TLBGuid *tlbguid, *tlbguid_next;
     ULONG ref;

EnterCriticalSection(&cache_section);
@@ -4792,72 +4796,67 @@ static ULONG WINAPI ITypeLib2_fnRelease( ITypeLib2 *iface)

TRACE("%p, refcount %lu.\n", iface, ref);

-    if (!ref)
-    {
-      TLBImpLib *pImpLib, *pImpLibNext;
-      TLBRefType *ref_type, *ref_type_next;
-      TLBString *tlbstr, *tlbstr_next;
-      TLBGuid *tlbguid, *tlbguid_next;
-      int i;
-
-      /* remove cache entry */
-      if(This->path)
-      {
-          TRACE("removing from cache list\n");
-          if(This->entry.next)
-              list_remove(&This->entry);
-          free(This->path);
-      }
-      TRACE(" destroying ITypeLib(%p)\n",This);
+    if (ref) {
+        LeaveCriticalSection(&cache_section);
+        return ref;
+    }

-      LIST_FOR_EACH_ENTRY_SAFE(tlbstr, tlbstr_next, &This->string_list, TLBString, entry) {
-          list_remove(&tlbstr->entry);
-          SysFreeString(tlbstr->str);
-          free(tlbstr);
-      }
+    /* remove cache entry */
+    if(This->path) {
+        TRACE("removing from cache list\n");
+        if(This->entry.next)
+            list_remove(&This->entry);
+        free(This->path);
+    }
+    TRACE(" destroying ITypeLib(%p)\n",This);

-      LIST_FOR_EACH_ENTRY_SAFE(tlbstr, tlbstr_next, &This->name_list, TLBString, entry) {
-          list_remove(&tlbstr->entry);
-          SysFreeString(tlbstr->str);
-          free(tlbstr);
-      }
+    LIST_FOR_EACH_ENTRY_SAFE(tlbstr, tlbstr_next, &This->string_list, TLBString, entry) {
+        list_remove(&tlbstr->entry);
+        SysFreeString(tlbstr->str);
+        free(tlbstr);
+    }

-      LIST_FOR_EACH_ENTRY_SAFE(tlbguid, tlbguid_next, &This->guid_list, TLBGuid, entry) {
-          list_remove(&tlbguid->entry);
-          free(tlbguid);
-      }
+    LIST_FOR_EACH_ENTRY_SAFE(tlbstr, tlbstr_next, &This->name_list, TLBString, entry) {
+        list_remove(&tlbstr->entry);
+        SysFreeString(tlbstr->str);
+        free(tlbstr);
+    }

-      TLB_FreeCustData(&This->custdata_list);
+    LIST_FOR_EACH_ENTRY_SAFE(tlbguid, tlbguid_next, &This->guid_list, TLBGuid, entry) {
+        list_remove(&tlbguid->entry);
+        free(tlbguid);
+    }

-      for (i = 0; i < This->ctTypeDesc; i++)
-          if (This->pTypeDesc[i].vt == VT_CARRAY)
-              free(This->pTypeDesc[i].lpadesc);
+    TLB_FreeCustData(&This->custdata_list);

-      free(This->pTypeDesc);
+    for (int i = 0; i < This->ctTypeDesc; i++) {
+        if (This->pTypeDesc[i].vt == VT_CARRAY)
+            free(This->pTypeDesc[i].lpadesc);
+    }
+    free(This->pTypeDesc);

-      LIST_FOR_EACH_ENTRY_SAFE(pImpLib, pImpLibNext, &This->implib_list, TLBImpLib, entry)
-      {
-          if (pImpLib->pImpTypeLib)
-              ITypeLib2_Release(&pImpLib->pImpTypeLib->ITypeLib2_iface);
-          SysFreeString(pImpLib->name);
+    LIST_FOR_EACH_ENTRY_SAFE(pImpLib, pImpLibNext, &This->implib_list, TLBImpLib, entry)
+    {
+        if (pImpLib->pImpTypeLib)
+            ITypeLib2_Release(&pImpLib->pImpTypeLib->ITypeLib2_iface);
+        SysFreeString(pImpLib->name);

-          list_remove(&pImpLib->entry);
-          free(pImpLib);
-      }
+        list_remove(&pImpLib->entry);
+        free(pImpLib);
+    }

-      LIST_FOR_EACH_ENTRY_SAFE(ref_type, ref_type_next, &This->ref_list, TLBRefType, entry)
-      {
-          list_remove(&ref_type->entry);
-          free(ref_type);
-      }
+    LIST_FOR_EACH_ENTRY_SAFE(ref_type, ref_type_next, &This->ref_list, TLBRefType, entry)
+    {
+        list_remove(&ref_type->entry);
+        free(ref_type);
+    }

-      for (i = 0; i < This->TypeInfoCount; ++i){
-          free(This->typeinfos[i]->tdescAlias);
-          ITypeInfoImpl_Destroy(This->typeinfos[i]);
-      }
-      free(This->typeinfos);
-      free(This);
+    for (int i = 0; i < This->TypeInfoCount; ++i) {
+        free(This->typeinfos[i]->tdescAlias);
+        ITypeInfoImpl_Destroy(This->typeinfos[i]);
     }
+    free(This->typeinfos);
+    free(This);

LeaveCriticalSection(&cache_section);
     return ref;