[PATCH v2 0/2] MR1774: Coverity fixes

[Fabian Maurer (＠DarkShadow44)]

Signed-off-by: Fabian Maurer

--
  v2: wldap32: Free resource in error case (Coverity)

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

[PATCH v2 1/2] urlon: Pass correct parameter to realloc (Coverity)

[Fabian Maurer]

From: Fabian Maurer

Signed-off-by: Fabian Maurer 
---
 dlls/urlmon/sec_mgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/urlmon/sec_mgr.c b/dlls/urlmon/sec_mgr.c
index e6d1864b1e7..0d81e4dfda9 100644
--- a/dlls/urlmon/sec_mgr.c
+++ b/dlls/urlmon/sec_mgr.c
@@ -1276,7 +1276,7 @@ static LPDWORD build_zonemap_from_reg(void)
             if (used == allocated) {
                 LPDWORD new_data;

-                new_data = realloc(data, allocated * sizeof(DWORD));
+                new_data = realloc(data, allocated * 2 * sizeof(DWORD));
                 if (!new_data)
                     goto cleanup;
                 memset(new_data + allocated, 0, allocated * sizeof(DWORD));

[PATCH v2 2/2] wldap32: Free resource in error case (Coverity)

[Fabian Maurer]

From: Fabian Maurer

---
 dlls/wldap32/misc.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/dlls/wldap32/misc.c b/dlls/wldap32/misc.c
index edf1f196353..97eb46141e1 100644
--- a/dlls/wldap32/misc.c
+++ b/dlls/wldap32/misc.c
@@ -413,10 +413,15 @@ ULONG CDECL WLDAP32_ldap_result( LDAP *ld, ULONG msgid, ULONG all, struct l_time

ret = ldap_result( CTX(ld), msgid, all, timeout ? &timeval : NULL, &msgU );
     }
-    if (msgU && (msg = calloc( 1, sizeof(*msg) )))
+    if (msgU)
     {
-        MSG(msg) = msgU;
-        *res = msg;
+        if ((msg = calloc( 1, sizeof(*msg) )))
+        {
+            MSG(msg) = msgU;
+            *res = msg;
+        }
+        else
+            free (msgU);
     }

return ret;

[Alex Henrie (＠alexhenrie)]

Alex Henrie (@alexhenrie) commented about dlls/urlmon/sec_mgr.c:
>          if (used == allocated) {
>              LPDWORD new_data;
>
> -
>            new_data = realloc(data, allocated * sizeof(DWORD));
> -
>            new_data = realloc(data, allocated * 2 * sizeof(DWORD));

Thank you for catching this (I'm surprised the tests didn't). The patch is correct, but please amend the commit message to say "urlmon" instead of "urlon".

[Alex Henrie (＠alexhenrie)]

Alex Henrie (@alexhenrie) commented about dlls/wldap32/misc.c:
>      ret = ldap_result( CTX(ld), msgid, all, timeout ? &timeval : NULL, &msgU );
>  }
> - if (msgU && (msg = calloc( 1, sizeof(*msg) )))
> - if (msgU)
> {
> -
>    MSG(msg) = msgU;
> -
>    *res = msg;
> -
>    if ((msg = calloc( 1, sizeof(*msg) )))
> -
>    {
> -
>        MSG(msg) = msgU;
> -
>        *res = msg;
> -
>    }
> -
>    else
> -
>        free (msgU);

Please use a consistent style here, namely `free( msgU )` instead of `free (msgU)`. Also, shouldn't this function return WLDAP32_LDAP_NO_MEMORY if calloc fails? I think the most clear way to handle the errors would be:

```
if (!msgU)
    return ret;

if (!(msg = calloc( 1, sizeof(*msg) )))
{
    free( msgU );
    return WLDAP32_LDAP_NO_MEMORY;
}

MSG(msg) = msgU;
*res = msg;
return ret;
```

[Fabian Maurer (＠DarkShadow44)]

On Mon Dec 12 02:53:32 2022 +0000, Alex Henrie wrote:
> Please use a consistent style here, namely `free( msgU )` instead of
> `free (msgU)`. Also, shouldn't this function return
> WLDAP32_LDAP_NO_MEMORY if calloc fails? I think the most clear way to
> handle the errors would be:
> if (!msgU)
>     return ret;
> if (!(msg = calloc( 1, sizeof(*msg) )))
> {
>     free( msgU );
>     return WLDAP32_LDAP_NO_MEMORY;
> }
> MSG(msg) = msgU;
> *res = msg;
> return ret;

Fixed the style issue.\
I didn't want to change the behavior, only fix the potential memory leak. There's a few other functions that also use calloc/malloc, they have the same behavior if allocation fails. If we change that here, we should probably adjust those as well.\
Do you want me to add those changes into this MR?