[PATCH v2 0/2] MR1774: Coverity fixes

[Fabian Maurer (＠DarkShadow44)]

Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>

--
  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 <dark.shadow4(a)web.de>

Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
---
 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));
-- 
GitLab


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

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

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

---
 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;
-- 
GitLab

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

[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".

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

[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;
```

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

[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?

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