[PATCH 0/3] MR1461: ieframe: Remove function heap_strdupW.

[PATCH 1/3] ieframe: Use standard C functions for memory allocation in dochost.c.

[Alex Henrie]

From: Alex Henrie

---
 dlls/ieframe/dochost.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/dlls/ieframe/dochost.c b/dlls/ieframe/dochost.c
index e73659f1736..97b86d481b9 100644
--- a/dlls/ieframe/dochost.c
+++ b/dlls/ieframe/dochost.c
@@ -269,7 +269,7 @@ static void ready_state_task_destr(task_header_t *_task)
     ready_state_task_t *task = (ready_state_task_t*)_task;

IUnknown_Release(task->doc);
-    heap_free(task);
+    free(task);
 }

static void ready_state_proc(DocHost *This, task_header_t *_task)
@@ -282,7 +282,7 @@ static void ready_state_proc(DocHost *This, task_header_t *_task)

static void push_ready_state_task(DocHost *This, READYSTATE ready_state)
 {
-    ready_state_task_t *task = heap_alloc(sizeof(ready_state_task_t));
+    ready_state_task_t *task = malloc(sizeof(ready_state_task_t));

IUnknown_AddRef(This->document);
     task->doc = This->document;
@@ -293,7 +293,7 @@ static void push_ready_state_task(DocHost *This, READYSTATE ready_state)

static void object_available_task_destr(task_header_t *task)
 {
-    heap_free(task);
+    free(task);
 }

static void object_available_proc(DocHost *This, task_header_t *task)
@@ -331,7 +331,7 @@ HRESULT dochost_object_available(DocHost *This, IUnknown *doc)

/* FIXME: Call SetAdvise */

-    task = heap_alloc(sizeof(*task));
+    task = malloc(sizeof(*task));
     push_dochost_task(This, task, object_available_proc, object_available_task_destr, FALSE);

hres = get_doc_ready_state(This, &ready_state);
@@ -388,7 +388,7 @@ static void free_travellog_entry(travellog_entry_t *entry)
         IStream_Release(entry->stream);
         entry->stream = NULL;
     }
-    heap_free(entry->url);
+    free(entry->url);
     entry->url = NULL;
 }

@@ -436,7 +436,7 @@ static void update_travellog(DocHost *This)
     }

if(!This->travellog.log) {
-        This->travellog.log = heap_alloc(4 * sizeof(*This->travellog.log));
+        This->travellog.log = malloc(4 * sizeof(*This->travellog.log));
         if(!This->travellog.log)
             return;

@@ -444,7 +444,7 @@ static void update_travellog(DocHost *This)
     }else if(This->travellog.size < This->travellog.position+1) {
         travellog_entry_t *new_travellog;

-        new_travellog = heap_realloc(This->travellog.log, This->travellog.size*2*sizeof(*This->travellog.log));
+        new_travellog = realloc(This->travellog.log, This->travellog.size * 2 * sizeof(*This->travellog.log));
         if(!new_travellog)
             return;

@@ -460,7 +460,7 @@ static void update_travellog(DocHost *This)

new_entry = This->travellog.log + This->travellog.position;

-    new_entry->url = heap_strdupW(This->url);
+    new_entry->url = wcsdup(This->url);
     TRACE("Adding %s at %d\n", debugstr_w(This->url), This->travellog.position);
     if(!new_entry->url)
         return;
@@ -1169,7 +1169,7 @@ void DocHost_Release(DocHost *This)

while(This->travellog.length)
         free_travellog_entry(This->travellog.log + --This->travellog.length);
-    heap_free(This->travellog.log);
+    free(This->travellog.log);

-    heap_free(This->url);
+    free(This->url);
 }

[PATCH 2/3] ieframe: Use standard C functions for memory allocation in navigate.c.

[Alex Henrie]

From: Alex Henrie

---
 dlls/ieframe/navigate.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/dlls/ieframe/navigate.c b/dlls/ieframe/navigate.c
index cb72464914d..8f8b1f3f192 100644
--- a/dlls/ieframe/navigate.c
+++ b/dlls/ieframe/navigate.c
@@ -132,14 +132,14 @@ HRESULT set_dochost_url(DocHost *This, const WCHAR *url)
     WCHAR *new_url;

if(url) {
-        new_url = heap_strdupW(url);
+        new_url = wcsdup(url);
         if(!new_url)
             return E_OUTOFMEMORY;
     }else {
         new_url = NULL;
     }

-    heap_free(This->url);
+    free(This->url);
     This->url = new_url;

This->container_vtbl->set_url(This, This->url);
@@ -215,7 +215,7 @@ static ULONG WINAPI BindStatusCallback_Release(IBindStatusCallback *iface)
             GlobalFree(This->post_data);
         SysFreeString(This->headers);
         SysFreeString(This->url);
-        heap_free(This);
+        free(This);
     }

return ref;
@@ -546,7 +546,7 @@ static const IHttpSecurityVtbl HttpSecurityVtbl = {
 static BindStatusCallback *create_callback(DocHost *doc_host, LPCWSTR url, PBYTE post_data,
         ULONG post_data_len, LPCWSTR headers)
 {
-    BindStatusCallback *ret = heap_alloc(sizeof(BindStatusCallback));
+    BindStatusCallback *ret = malloc(sizeof(BindStatusCallback));

ret->IBindStatusCallback_iface.lpVtbl = &BindStatusCallbackVtbl;
     ret->IHttpNegotiate_iface.lpVtbl      = &HttpNegotiateVtbl;
@@ -780,7 +780,7 @@ static void doc_navigate_task_destr(task_header_t *t)
     SysFreeString(task->headers);
     if(task->post_data)
         SafeArrayDestroy(task->post_data);
-    heap_free(task);
+    free(task);
 }

static void doc_navigate_proc(DocHost *This, task_header_t *t)
@@ -819,7 +819,7 @@ static HRESULT async_doc_navigate(DocHost *This, LPCWSTR url, LPCWSTR headers, P

TRACE("%s\n", debugstr_w(url));

-    task = heap_alloc_zero(sizeof(*task));
+    task = calloc(1, sizeof(*task));
     if(!task)
         return E_OUTOFMEMORY;

@@ -920,7 +920,7 @@ static void navigate_bsc_task_destr(task_header_t *t)
     task_navigate_bsc_t *task = (task_navigate_bsc_t*)t;

IBindStatusCallback_Release(&task->bsc->IBindStatusCallback_iface);
-    heap_free(task);
+    free(task);
 }

static void navigate_bsc_proc(DocHost *This, task_header_t *t)
@@ -993,7 +993,7 @@ HRESULT navigate_url(DocHost *This, LPCWSTR url, const VARIANT *Flags,
     }else {
         task_navigate_bsc_t *task;

-        task = heap_alloc(sizeof(*task));
+        task = malloc(sizeof(*task));
         task->bsc = create_callback(This, url, post_data, post_data_len, headers);
         push_dochost_task(This, &task->header, navigate_bsc_proc, navigate_bsc_task_destr, This->url == NULL);
     }

[PATCH 3/3] ieframe: Remove unused function heap_strdupW.

[Alex Henrie]

From: Alex Henrie

---
 dlls/ieframe/ieframe.h | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/dlls/ieframe/ieframe.h b/dlls/ieframe/ieframe.h
index 633906a70ca..a1e3a608dfa 100644
--- a/dlls/ieframe/ieframe.h
+++ b/dlls/ieframe/ieframe.h
@@ -340,22 +340,6 @@ static inline void unlock_module(void) {
     InterlockedDecrement(&module_ref);
 }

-static inline LPWSTR heap_strdupW(LPCWSTR str)
-{
-    LPWSTR ret = NULL;
-
-    if(str) {
-        DWORD size;
-
-        size = (lstrlenW(str)+1)*sizeof(WCHAR);
-        ret = heap_alloc(size);
-        if(ret)
-            memcpy(ret, str, size);
-    }
-
-    return ret;
-}
-
 static inline LPWSTR co_strdupW(LPCWSTR str)
 {
     WCHAR *ret = CoTaskMemAlloc((lstrlenW(str) + 1)*sizeof(WCHAR));

[PATCH 3/3] ieframe: Remove unused function heap_strdupW.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=126523

Your paranoid android.

=== debian11 (32 bit report) ===

d3drm:
d3drm.c:5843: Test failed: Cannot create IM device, skipping tests.

=== debian11 (build log) ===

0544:err:winediag:d3d_device_create The application wants to create a Direct3D device, but the current DirectDrawRenderer does not support this.
0544:err:winediag:d3d_device_create The application wants to create a Direct3D device, but the current DirectDrawRenderer does not support this.
0544:err:winediag:d3d_device_create The application wants to create a Direct3D device, but the current DirectDrawRenderer does not support this.

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/ieframe/navigate.c:
>  WCHAR *new_url;
>
>  if(url) {
> -
>    new_url = heap_strdupW(url);
> -
>    new_url = wcsdup(url);
>    if(!new_url)
>        return E_OUTOFMEMORY;
> }else {
>        new_url = NULL;
>    }
> - heap_free(This->url);
> - free(This->url);

This does not match `DocHost_Release`. It's not the first such case, the way you selectively convert allocations is both more error-prone and harder to review than needed.

[Alex Henrie (＠alexhenrie)]

On Mon Nov 21 13:21:54 2022 +0000, Jacek Caban wrote:
> This does not match `DocHost_Release`. It's not the first such case, the
> way you selectively convert allocations is both more error-prone and
> harder to review than needed.

Thanks for catching that. Would you prefer one big patch that converts the entire DLL all at once? I'm happy to do that, I just thought that big patches were frowned upon in the Wine project.

[Jacek Caban (＠jacek)]

On Mon Nov 21 15:16:22 2022 +0000, Alex Henrie wrote:
> Thanks for catching that. Would you prefer one big patch that converts
> the entire DLL all at once? I'm happy to do that, I just thought that
> big patches were frowned upon in the Wine project.

The main objective is to make patch easier to review. Small patches are usually a good way of achieving that, but not in this case. I think that module-wide patch would be better in this case.