[PATCH v2 0/1] MR4292: d3dcompiler_43/tests: Use CRT allocation functions.

[Alex Henrie (＠alexhenrie)]

--
  v2: d3dcompiler_43/tests: Use CRT allocation functions.

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

[PATCH v2 1/1] d3dcompiler_43/tests: Use CRT allocation functions.

[Alex Henrie]

From: Alex Henrie

---
 dlls/d3dcompiler_43/tests/asm.c       | 17 ++++++-----------
 dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 15 +++++++--------
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/dlls/d3dcompiler_43/tests/asm.c b/dlls/d3dcompiler_43/tests/asm.c
index 6d3e7013017..55d84213e1e 100644
--- a/dlls/d3dcompiler_43/tests/asm.c
+++ b/dlls/d3dcompiler_43/tests/asm.c
@@ -1460,23 +1460,20 @@ static HRESULT WINAPI testD3DInclude_open(ID3DInclude *iface, D3D_INCLUDE_TYPE i

if (!strcmp(filename, "incl.vsh"))
     {
-        buffer = HeapAlloc(GetProcessHeap(), 0, sizeof(include));
-        CopyMemory(buffer, include, sizeof(include));
+        buffer = strdup(include);
         *bytes = sizeof(include);
         ok(!parent_data, "Wrong parent_data value.\n");
     }
     else if (!strcmp(filename, "incl2.vsh"))
     {
-        buffer = HeapAlloc(GetProcessHeap(), 0, sizeof(include2));
-        CopyMemory(buffer, include2, sizeof(include2));
+        buffer = strdup(include2);
         *bytes = sizeof(include2);
         ok(!parent_data, "Wrong parent_data value.\n");
         ok(include_type == D3D_INCLUDE_LOCAL, "Wrong include type %d.\n", include_type);
     }
     else if (!strcmp(filename, "incl3.vsh"))
     {
-        buffer = HeapAlloc(GetProcessHeap(), 0, sizeof(include3));
-        CopyMemory(buffer, include3, sizeof(include3));
+        buffer = strdup(include3);
         *bytes = sizeof(include3);
         /* Also check for the correct parent_data content */
         ok(parent_data != NULL
@@ -1486,16 +1483,14 @@ static HRESULT WINAPI testD3DInclude_open(ID3DInclude *iface, D3D_INCLUDE_TYPE i
     }
     else if (!strcmp(filename, "incl4.vsh"))
     {
-        buffer = HeapAlloc(GetProcessHeap(), 0, sizeof(include4));
-        CopyMemory(buffer, include4, sizeof(include4));
+        buffer = strdup(include4);
         *bytes = sizeof(include4);
         ok(parent_data == NULL, "Wrong parent_data value.\n");
         ok(include_type == D3D_INCLUDE_SYSTEM, "Wrong include type %d.\n", include_type);
     }
     else if (!strcmp(filename, "includes/incl.vsh"))
     {
-        buffer = HeapAlloc(GetProcessHeap(), 0, sizeof(include));
-        CopyMemory(buffer, include, sizeof(include));
+        buffer = strdup(include);
         *bytes = sizeof(include);
         ok(!parent_data, "Wrong parent_data value.\n");
     }
@@ -1512,7 +1507,7 @@ static HRESULT WINAPI testD3DInclude_open(ID3DInclude *iface, D3D_INCLUDE_TYPE i

static HRESULT WINAPI testD3DInclude_close(ID3DInclude *iface, const void *data)
 {
-    HeapFree(GetProcessHeap(), 0, (void *)data);
+    free((void *)data);
     return S_OK;
 }

diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c
index c6e7aa367f0..eed96db7248 100644
--- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c
+++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c
@@ -18,7 +18,6 @@
  */
 #define COBJMACROS
 #include "wine/test.h"
-#include "wine/heap.h"
 #include "d3dx9.h"
 #include "d3dcompiler.h"

@@ -1515,26 +1514,26 @@ static HRESULT WINAPI test_d3dinclude_open(ID3DInclude *iface, D3D_INCLUDE_TYPE

if (!strcmp(filename, "include1.h"))
     {
-        buffer = heap_alloc(strlen(include1));
-        CopyMemory(buffer, include1, strlen(include1));
+        buffer = strdup(include1);
         *bytes = strlen(include1);
+        buffer[*bytes] = '$'; /* everything should still work without a null terminator */
         ok(include_type == D3D_INCLUDE_LOCAL, "Unexpected include type %d.\n", include_type);
         ok(!strncmp(include2, parent_data, strlen(include2)) || !strncmp(include3, parent_data, strlen(include3)),
                 "Unexpected parent_data value.\n");
     }
     else if (!strcmp(filename, "include\include2.h"))
     {
-        buffer = heap_alloc(strlen(include2));
-        CopyMemory(buffer, include2, strlen(include2));
+        buffer = strdup(include2);
         *bytes = strlen(include2);
+        buffer[*bytes] = '$'; /* everything should still work without a null terminator */
         ok(!parent_data, "Unexpected parent_data value.\n");
         ok(include_type == D3D_INCLUDE_LOCAL, "Unexpected include type %d.\n", include_type);
     }
     else if (!strcmp(filename, "include\include3.h"))
     {
-        buffer = heap_alloc(strlen(include3));
-        CopyMemory(buffer, include3, strlen(include3));
+        buffer = strdup(include3);
         *bytes = strlen(include3);
+        buffer[*bytes] = '$'; /* everything should still work without a null terminator */
         ok(!parent_data, "Unexpected parent_data value.\n");
         ok(include_type == D3D_INCLUDE_LOCAL, "Unexpected include type %d.\n", include_type);
     }
@@ -1550,7 +1549,7 @@ static HRESULT WINAPI test_d3dinclude_open(ID3DInclude *iface, D3D_INCLUDE_TYPE

static HRESULT WINAPI test_d3dinclude_close(ID3DInclude *iface, const void *data)
 {
-    heap_free((void *)data);
+    free((void *)data);
     return S_OK;
 }

[Alex Henrie (＠alexhenrie)]

On Thu Nov 16 06:55:21 2023 +0000, Matteo Bruni wrote:
> Sorry, I missed this MR somehow.
> It seems to me that this patch does it backwards, in a way.
> Specifically, the HeapAlloc(sizeof(string)) are replaced with malloc(),
> while the heap_alloc(strlen(string)) + CopyMemory() become strdup(). The
> former is okay, although we could replace the whole memory allocation +
> string copy with strdup() just fine.
> The latter changes behavior slightly, in that it was previously copying
> the string without the string terminator while now it allocates and
> copies that extra byte. Now, to be fair, this effectively "depended" on
> the arbitrary value that happens to be on that (last + 1) byte, but then
> again it should probably be improved anyway to allocate the extra byte
> and explicitly set it to some value != 0.

Thanks for the feedback! I've amended the patch to use strdup in asm.c and explicitly remove the null terminators in hlsl_d3d9.c.

[PATCH v2 0/1] MR4292: d3dcompiler_43/tests: Use CRT allocation functions. - approved

[Matteo Bruni (＠Mystral)]

This merge request was approved by Matteo Bruni.