[PATCH v7 0/5] MR8703: combase: Implement HSTRING marshaling functions.

[Vibhav Pant (＠vibhavp)]

Add tests + implementations for the `HSTRING_User{Size, Marshal, Unmarshal, Free}` methods, required by proxies generated for interfaces that utilize HSTRING.

--
  v7: combase: Implement HSTRING_User{Unmarshal, Free}.
      combase: Implement HSTRING_UserMarshal.

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

[PATCH v7 1/5] combase: Add stubs for HSTRING_User{Size, Marshal, Unmarshal, Free}.

[Vibhav Pant]

From: Vibhav Pant

---
 dlls/combase/combase.spec |  8 ++++----
 dlls/combase/usrmarshal.c | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/dlls/combase/combase.spec b/dlls/combase/combase.spec
index 7e2d34447a3..6d992d4a675 100644
--- a/dlls/combase/combase.spec
+++ b/dlls/combase/combase.spec
@@ -220,13 +220,13 @@
 @ stdcall HPALETTE_UserMarshal(ptr ptr ptr)
 @ stdcall HPALETTE_UserSize(ptr long ptr)
 @ stdcall HPALETTE_UserUnmarshal(ptr ptr ptr)
-@ stub HSTRING_UserFree
+@ stdcall HSTRING_UserFree(ptr ptr)
 @ stub -arch=win64 HSTRING_UserFree64
-@ stub HSTRING_UserMarshal
+@ stdcall HSTRING_UserMarshal(ptr ptr ptr)
 @ stub -arch=win64 HSTRING_UserMarshal64
-@ stub HSTRING_UserSize
+@ stdcall HSTRING_UserSize(ptr long ptr)
 @ stub -arch=win64 HSTRING_UserSize64
-@ stub HSTRING_UserUnmarshal
+@ stdcall HSTRING_UserUnmarshal(ptr ptr ptr)
 @ stub -arch=win64 HSTRING_UserUnmarshal64
 @ stdcall HWND_UserFree(ptr ptr)
 @ stdcall HWND_UserMarshal(ptr ptr ptr)
diff --git a/dlls/combase/usrmarshal.c b/dlls/combase/usrmarshal.c
index 87513d335bf..b3321d8baa8 100644
--- a/dlls/combase/usrmarshal.c
+++ b/dlls/combase/usrmarshal.c
@@ -22,6 +22,7 @@
 #define COBJMACROS
 #include "ole2.h"

+#include "inspectable.h"
 #include "wine/debug.h"

WINE_DEFAULT_DEBUG_CHANNEL(ole);
@@ -1025,3 +1026,38 @@ void WINAPI WdtpInterfacePointer_UserFree(IUnknown *punk)
     TRACE("%p.\n", punk);
     if (punk) IUnknown_Release(punk);
 }
+
+/******************************************************************************
+ *           HSTRING_UserSize (combase.@)
+ */
+ULONG __RPC_USER HSTRING_UserSize(ULONG *flags, ULONG start, HSTRING *str)
+{
+    FIXME("%p, %lu, %p: stub\n", flags, start, str);
+    return start;
+}
+
+/******************************************************************************
+ *           HSTRING_UserMarshal (combase.@)
+ */
+BYTE * __RPC_USER HSTRING_UserMarshal(ULONG *flags, BYTE *buf, HSTRING *str)
+{
+    FIXME("%p, %p, %p: stub\n", flags, buf, str);
+    return buf;
+}
+
+/******************************************************************************
+ *           HSTRING_UserUnmarshal (combase.@)
+ */
+BYTE * __RPC_USER HSTRING_UserUnmarshal(ULONG *flags, BYTE *buf, HSTRING *str)
+{
+    FIXME("%p, %p, %p: stub\n", flags, buf, str);
+    return buf;
+}
+
+/******************************************************************************
+ *           HSTRING_UserFree (combase.@)
+ */
+void __RPC_USER HSTRING_UserFree(ULONG *flags, HSTRING *str)
+{
+    FIXME("%p, %p: stub\n", flags, str);
+}

[PATCH v7 2/5] combase/tests: Add tests for HSTRING marshaling methods.

[Vibhav Pant]

From: Vibhav Pant

---
 dlls/combase/tests/string.c | 212 ++++++++++++++++++++++++++++++++++++
 1 file changed, 212 insertions(+)

diff --git a/dlls/combase/tests/string.c b/dlls/combase/tests/string.c
index ceb8ffa6e09..b536f75e102 100644
--- a/dlls/combase/tests/string.c
+++ b/dlls/combase/tests/string.c
@@ -24,6 +24,7 @@
 #include "winbase.h"
 #include "winerror.h"
 #include "winstring.h"
+#include "inspectable.h"

#include "wine/test.h"

@@ -545,6 +546,216 @@ static void test_hstring_struct(void)
     ok(WindowsDeleteString(str) == S_OK, "Failed to delete string ref.\n");
 }

+static void * WINAPI user_allocate(SIZE_T size)
+{
+    ok(0, "unexpected user_allocate call\n");
+    return CoTaskMemAlloc(size);
+}
+
+static void WINAPI user_free(void *p)
+{
+    ok(0, "unexpected user_free call\n");
+    CoTaskMemFree(p);
+}
+
+static void init_user_marshal_cb(USER_MARSHAL_CB *umcb,
+                                 PMIDL_STUB_MESSAGE stub_msg,
+                                 PRPC_MESSAGE rpc_msg, unsigned char *buffer,
+                                 unsigned int size, MSHCTX context)
+{
+    memset(rpc_msg, 0, sizeof(*rpc_msg));
+    rpc_msg->Buffer = buffer;
+    rpc_msg->BufferLength = size;
+
+    memset(stub_msg, 0, sizeof(*stub_msg));
+    stub_msg->RpcMsg = rpc_msg;
+    stub_msg->Buffer = buffer;
+    stub_msg->pfnAllocate = user_allocate;
+    stub_msg->pfnFree = user_free;
+
+    memset(umcb, 0, sizeof(*umcb));
+    umcb->Flags = MAKELONG(context, NDR_LOCAL_DATA_REPRESENTATION);
+    umcb->pStubMsg = stub_msg;
+    umcb->Signature = USER_MARSHAL_CB_SIGNATURE;
+    umcb->CBType = buffer ? USER_MARSHAL_CB_UNMARSHALL : USER_MARSHAL_CB_BUFFER_SIZE;
+}
+
+#define ALIGNED_LENGTH(_Len, _Align) (((_Len)+(_Align))&~(_Align))
+#define ALIGNED_POINTER(_Ptr, _Align) ((LPVOID)ALIGNED_LENGTH((ULONG_PTR)(_Ptr), _Align))
+
+#pragma pack(push, 1)
+struct hstring_wire
+{
+    /* Marshaling context, either WDT_INPROC64_CALL or WDT_INPROC_CALL. */
+    ULONG context;
+    union {
+        struct {
+            ULONG size;
+            WCHAR data[];
+        } buf;
+        struct {
+#ifdef _WIN64
+            ULONG pad;
+#endif
+            HSTRING str;
+        };
+    };
+};
+#pragma pack(pop)
+
+static void test_marshal(void)
+{
+    static const WCHAR str_buf[] = L"marshal_test1";
+    static const ULONG str_len = ARRAY_SIZE(str_buf) - 1;
+    static const ULONG str_bytes = str_len * sizeof(WCHAR);
+#ifdef _WIN64
+    const ULONG exp_prefix = WDT_INPROC64_CALL;
+#else
+    const ULONG exp_prefix = WDT_INPROC_CALL;
+#endif
+    HSTRING str, str_empty = NULL, str2 = NULL;
+    const struct hstring_wire *wire;
+    BYTE *buffer = calloc(1, 80);
+    MIDL_STUB_MESSAGE stub_msg;
+    USER_MARSHAL_CB umcb;
+    ULONG size, exp_size;
+    RPC_MESSAGE rpc_msg;
+    INT32 order = -1;
+    BYTE *next;
+    HRESULT hr;
+
+    hr = WindowsCreateString(str_buf, wcslen(str_buf), &str);
+    ok(hr == S_OK, "got hr %#lx\n", hr);
+
+    /* INPROC marshaling */
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_INPROC);
+    size = HSTRING_UserSize(&umcb.Flags, 0, &str);
+    exp_size = sizeof(struct hstring_wire);
+    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
+    next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    wire = (struct hstring_wire *)buffer;
+    todo_wine ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
+    /* INPROC marshaling just consists of increasing the refcount and copying the address. */
+    todo_wine ok(wire->str == str, "got unexpected address %p\n", wire->str);
+    next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    todo_wine ok(str2 == str, "got str2 %p != %p\n", str2, str);
+    HSTRING_UserFree(&umcb.Flags, &str2);
+
+    /* Test alignment */
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_INPROC);
+    size = HSTRING_UserSize(&umcb.Flags, 1, &str);
+    exp_size = ALIGNED_LENGTH(1 + sizeof(struct hstring_wire), 7);
+    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    memset(buffer, 0, 80);
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
+    next = HSTRING_UserMarshal(&umcb.Flags, &buffer[1], &str);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    wire = ALIGNED_POINTER(&buffer[1], 7);
+    todo_wine ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
+    todo_wine ok(wire->str == str, "got unexpected address %p\n", wire->str);
+    next = HSTRING_UserUnmarshal(&umcb.Flags, &buffer[1], &str2);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    todo_wine ok(str2 == str, "got str2 %p != %p\n", str2, str);
+    HSTRING_UserFree(&umcb.Flags, &str2);
+
+    /* INPROC marshaling with empty/NULL HSTRING */
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_INPROC);
+    size = HSTRING_UserSize(&umcb.Flags, 0, &str_empty);
+    exp_size = sizeof(struct hstring_wire);
+    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    memset(buffer, 0xff, 80);
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
+    next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str_empty);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    wire = (struct hstring_wire *)buffer;
+    todo_wine ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
+    todo_wine ok(!wire->str, "got unexpected address %p\n", wire->str);
+    str2 = NULL;
+    next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(!str2, "got str2 %p\n", str2);
+    HSTRING_UserFree(&umcb.Flags, &str2);
+
+    /* Out of process marshaling */
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
+    size = HSTRING_UserSize(&umcb.Flags, 0, &str);
+    exp_size = offsetof(struct hstring_wire, buf.data[str_len]);
+    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    memset(buffer, 0, 80);
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
+    next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    wire = (struct hstring_wire *)buffer;
+    todo_wine ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
+    if (size == exp_size)
+        todo_wine ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
+    str2 = NULL;
+    next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
+    wire = (struct hstring_wire *)buffer;
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    /* A new HSTRING should be allocated */
+    todo_wine_if(str2) ok(str2 != str, "got str2 %p\n", str2);
+    hr = WindowsCompareStringOrdinal(str2, str, &order);
+    ok(hr == S_OK, "got hr %#lx\n", hr);
+    todo_wine ok(!order, "got str2 %s != %s\n", debugstr_hstring(str2), debugstr_hstring(str));
+    HSTRING_UserFree(&umcb.Flags, &str2);
+
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
+    size = HSTRING_UserSize(&umcb.Flags, 1, &str);
+    exp_size = ALIGNED_LENGTH(1, 7) + offsetof(struct hstring_wire, buf.data[str_len]);
+    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    memset(buffer, 0, 80);
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
+    next = HSTRING_UserMarshal(&umcb.Flags, &buffer[1], &str);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    wire = ALIGNED_POINTER(&buffer[1], 7);
+    todo_wine ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
+    if (size == exp_size)
+        todo_wine ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
+    next = HSTRING_UserUnmarshal(&umcb.Flags, &buffer[1], &str2);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    todo_wine_if(str2) ok(str2 != str, "got str2 %p\n", str2);
+    order = -1;
+    hr = WindowsCompareStringOrdinal(str2, str, &order);
+    ok(hr == S_OK, "got hr %#lx\n", hr);
+    todo_wine ok(!order, "got str2 %s != %s\n", debugstr_hstring(str2), debugstr_hstring(str));
+    HSTRING_UserFree(&umcb.Flags, &str2);
+
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
+    size = HSTRING_UserSize(&umcb.Flags, 0, &str_empty);
+    exp_size = offsetof(struct hstring_wire, buf.data[0]);
+    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    memset(buffer, 0xff, 80);
+    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
+    next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str_empty);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    wire = (struct hstring_wire *)buffer;
+    todo_wine ok(!wire->buf.size, "got buf.size %lu\n", wire->buf.size);
+    str2 = NULL;
+    next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
+    if (size == exp_size)
+        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(!str2, "got str2 %p\n", str2);
+    HSTRING_UserFree(&umcb.Flags, &str2);
+
+    WindowsDeleteString(str);
+    free(buffer);
+}
+
 START_TEST(string)
 {
     test_create_delete();
@@ -556,4 +767,5 @@ START_TEST(string)
     test_compare();
     test_trim();
     test_hstring_struct();
+    test_marshal();
 }

[PATCH v7 3/5] combase: Implement HSTRING_UserSize.

[Vibhav Pant]

From: Vibhav Pant

---
 dlls/combase/tests/string.c | 12 ++++++------
 dlls/combase/usrmarshal.c   | 28 +++++++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/dlls/combase/tests/string.c b/dlls/combase/tests/string.c
index b536f75e102..42cb7e1bb91 100644
--- a/dlls/combase/tests/string.c
+++ b/dlls/combase/tests/string.c
@@ -631,7 +631,7 @@ static void test_marshal(void)
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_INPROC);
     size = HSTRING_UserSize(&umcb.Flags, 0, &str);
     exp_size = sizeof(struct hstring_wire);
-    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str);
     if (size == exp_size)
@@ -650,7 +650,7 @@ static void test_marshal(void)
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_INPROC);
     size = HSTRING_UserSize(&umcb.Flags, 1, &str);
     exp_size = ALIGNED_LENGTH(1 + sizeof(struct hstring_wire), 7);
-    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
     memset(buffer, 0, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
     next = HSTRING_UserMarshal(&umcb.Flags, &buffer[1], &str);
@@ -669,7 +669,7 @@ static void test_marshal(void)
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_INPROC);
     size = HSTRING_UserSize(&umcb.Flags, 0, &str_empty);
     exp_size = sizeof(struct hstring_wire);
-    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
     memset(buffer, 0xff, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str_empty);
@@ -689,7 +689,7 @@ static void test_marshal(void)
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
     size = HSTRING_UserSize(&umcb.Flags, 0, &str);
     exp_size = offsetof(struct hstring_wire, buf.data[str_len]);
-    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
     memset(buffer, 0, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str);
@@ -714,7 +714,7 @@ static void test_marshal(void)
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
     size = HSTRING_UserSize(&umcb.Flags, 1, &str);
     exp_size = ALIGNED_LENGTH(1, 7) + offsetof(struct hstring_wire, buf.data[str_len]);
-    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
     memset(buffer, 0, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
     next = HSTRING_UserMarshal(&umcb.Flags, &buffer[1], &str);
@@ -737,7 +737,7 @@ static void test_marshal(void)
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
     size = HSTRING_UserSize(&umcb.Flags, 0, &str_empty);
     exp_size = offsetof(struct hstring_wire, buf.data[0]);
-    todo_wine ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
+    ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
     memset(buffer, 0xff, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str_empty);
diff --git a/dlls/combase/usrmarshal.c b/dlls/combase/usrmarshal.c
index b3321d8baa8..bd747d87ee8 100644
--- a/dlls/combase/usrmarshal.c
+++ b/dlls/combase/usrmarshal.c
@@ -22,6 +22,7 @@
 #define COBJMACROS
 #include "ole2.h"

+#include "winstring.h"
 #include "inspectable.h"
 #include "wine/debug.h"

@@ -1027,12 +1028,37 @@ void WINAPI WdtpInterfacePointer_UserFree(IUnknown *punk)
     if (punk) IUnknown_Release(punk);
 }

+#pragma pack(push, 1)
+struct hstring_wire
+{
+    ULONG context;
+    union {
+        struct {
+            ULONG size;
+            WCHAR data[];
+        } buf;
+        struct {
+#ifdef _WIN64
+            ULONG pad;
+#endif
+            HSTRING str;
+        };
+    };
+};
+#pragma pack(pop)
+
 /******************************************************************************
  *           HSTRING_UserSize (combase.@)
  */
 ULONG __RPC_USER HSTRING_UserSize(ULONG *flags, ULONG start, HSTRING *str)
 {
-    FIXME("%p, %lu, %p: stub\n", flags, start, str);
+    TRACE("%s, %lu, %s.\n", debugstr_user_flags(flags), start, debugstr_hstring(*str));
+
+    ALIGN_LENGTH(start, 7);
+    if (LOWORD(*flags) == MSHCTX_INPROC)
+        start += sizeof(struct hstring_wire);
+    else
+        start += offsetof(struct hstring_wire, buf.data[WindowsGetStringLen(*str)]);
     return start;
 }

[PATCH v7 4/5] combase: Implement HSTRING_UserMarshal.

[Vibhav Pant]

From: Vibhav Pant

---
 dlls/combase/tests/string.c | 42 +++++++++++++++----------------------
 dlls/combase/usrmarshal.c   | 33 ++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 26 deletions(-)

diff --git a/dlls/combase/tests/string.c b/dlls/combase/tests/string.c
index 42cb7e1bb91..43532d35f31 100644
--- a/dlls/combase/tests/string.c
+++ b/dlls/combase/tests/string.c
@@ -634,12 +634,11 @@ static void test_marshal(void)
     ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     wire = (struct hstring_wire *)buffer;
-    todo_wine ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
+    ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
     /* INPROC marshaling just consists of increasing the refcount and copying the address. */
-    todo_wine ok(wire->str == str, "got unexpected address %p\n", wire->str);
+    ok(wire->str == str, "got unexpected address %p\n", wire->str);
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
     if (size == exp_size)
         todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
@@ -654,11 +653,10 @@ static void test_marshal(void)
     memset(buffer, 0, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
     next = HSTRING_UserMarshal(&umcb.Flags, &buffer[1], &str);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     wire = ALIGNED_POINTER(&buffer[1], 7);
-    todo_wine ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
-    todo_wine ok(wire->str == str, "got unexpected address %p\n", wire->str);
+    ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
+    ok(wire->str == str, "got unexpected address %p\n", wire->str);
     next = HSTRING_UserUnmarshal(&umcb.Flags, &buffer[1], &str2);
     if (size == exp_size)
         todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
@@ -673,11 +671,10 @@ static void test_marshal(void)
     memset(buffer, 0xff, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_INPROC);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str_empty);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     wire = (struct hstring_wire *)buffer;
-    todo_wine ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
-    todo_wine ok(!wire->str, "got unexpected address %p\n", wire->str);
+    ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
+    ok(!wire->str, "got unexpected address %p\n", wire->str);
     str2 = NULL;
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
     if (size == exp_size)
@@ -693,12 +690,10 @@ static void test_marshal(void)
     memset(buffer, 0, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     wire = (struct hstring_wire *)buffer;
-    todo_wine ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
-    if (size == exp_size)
-        todo_wine ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
+    ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
+    ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
     str2 = NULL;
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
     wire = (struct hstring_wire *)buffer;
@@ -718,12 +713,10 @@ static void test_marshal(void)
     memset(buffer, 0, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
     next = HSTRING_UserMarshal(&umcb.Flags, &buffer[1], &str);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     wire = ALIGNED_POINTER(&buffer[1], 7);
-    todo_wine ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
-    if (size == exp_size)
-        todo_wine ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
+    ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
+    ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
     next = HSTRING_UserUnmarshal(&umcb.Flags, &buffer[1], &str2);
     if (size == exp_size)
         todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
@@ -741,10 +734,9 @@ static void test_marshal(void)
     memset(buffer, 0xff, 80);
     init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
     next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str_empty);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     wire = (struct hstring_wire *)buffer;
-    todo_wine ok(!wire->buf.size, "got buf.size %lu\n", wire->buf.size);
+    ok(!wire->buf.size, "got buf.size %lu\n", wire->buf.size);
     str2 = NULL;
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
     if (size == exp_size)
diff --git a/dlls/combase/usrmarshal.c b/dlls/combase/usrmarshal.c
index bd747d87ee8..f09fb5ebe5f 100644
--- a/dlls/combase/usrmarshal.c
+++ b/dlls/combase/usrmarshal.c
@@ -1067,7 +1067,38 @@ ULONG __RPC_USER HSTRING_UserSize(ULONG *flags, ULONG start, HSTRING *str)
  */
 BYTE * __RPC_USER HSTRING_UserMarshal(ULONG *flags, BYTE *buf, HSTRING *str)
 {
-    FIXME("%p, %p, %p: stub\n", flags, buf, str);
+    struct hstring_wire *wire;
+
+    TRACE("%s, %p, %s.\n", debugstr_user_flags(flags), buf, debugstr_hstring(*str));
+
+    if (LOWORD(*flags) == MSHCTX_DIFFERENTMACHINE)
+    {
+        FIXME("MSHCTX_DIFFERENTMACHINE is not supported yet.\n");
+        RpcRaiseException(RPC_S_INVALID_TAG);
+    }
+
+    wire = ALIGNED_POINTER(buf, 7);
+#ifdef _WIN64
+    wire->context = WDT_INPROC64_CALL;
+#else
+    wire->context = WDT_INPROC_CALL;
+#endif
+    if (LOWORD(*flags) == MSHCTX_INPROC)
+    {
+        WindowsDuplicateString(*str, &wire->str);
+        buf = (BYTE *)(&wire->str + 1);
+    }
+    else
+    {
+        const WCHAR *str_buf;
+        UINT32 len;
+
+        str_buf = WindowsGetStringRawBuffer(*str, &len);
+        wire->buf.size = len * sizeof(WCHAR);
+        memcpy(wire->buf.data, str_buf, wire->buf.size);
+        buf = (BYTE*)&wire->buf.data[len];
+    }
+
     return buf;
 }

[PATCH v7 5/5] combase: Implement HSTRING_User{Unmarshal, Free}.

[Vibhav Pant]

From: Vibhav Pant

---
 dlls/combase/tests/string.c | 30 ++++++++++++------------------
 dlls/combase/usrmarshal.c   | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 20 deletions(-)

diff --git a/dlls/combase/tests/string.c b/dlls/combase/tests/string.c
index 43532d35f31..bb9005af52c 100644
--- a/dlls/combase/tests/string.c
+++ b/dlls/combase/tests/string.c
@@ -640,9 +640,8 @@ static void test_marshal(void)
     /* INPROC marshaling just consists of increasing the refcount and copying the address. */
     ok(wire->str == str, "got unexpected address %p\n", wire->str);
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
-    todo_wine ok(str2 == str, "got str2 %p != %p\n", str2, str);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(str2 == str, "got str2 %p != %p\n", str2, str);
     HSTRING_UserFree(&umcb.Flags, &str2);

/* Test alignment */
@@ -658,9 +657,8 @@ static void test_marshal(void)
     ok(wire->context == exp_prefix, "got unexpected prefix %#lx != %#lx\n", wire->context, exp_prefix);
     ok(wire->str == str, "got unexpected address %p\n", wire->str);
     next = HSTRING_UserUnmarshal(&umcb.Flags, &buffer[1], &str2);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
-    todo_wine ok(str2 == str, "got str2 %p != %p\n", str2, str);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(str2 == str, "got str2 %p != %p\n", str2, str);
     HSTRING_UserFree(&umcb.Flags, &str2);

/* INPROC marshaling with empty/NULL HSTRING */
@@ -677,8 +675,7 @@ static void test_marshal(void)
     ok(!wire->str, "got unexpected address %p\n", wire->str);
     str2 = NULL;
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     ok(!str2, "got str2 %p\n", str2);
     HSTRING_UserFree(&umcb.Flags, &str2);

@@ -697,13 +694,12 @@ static void test_marshal(void)
     str2 = NULL;
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
     wire = (struct hstring_wire *)buffer;
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     /* A new HSTRING should be allocated */
-    todo_wine_if(str2) ok(str2 != str, "got str2 %p\n", str2);
+    ok(str2 != str, "got str2 %p\n", str2);
     hr = WindowsCompareStringOrdinal(str2, str, &order);
     ok(hr == S_OK, "got hr %#lx\n", hr);
-    todo_wine ok(!order, "got str2 %s != %s\n", debugstr_hstring(str2), debugstr_hstring(str));
+    ok(!order, "got str2 %s != %s\n", debugstr_hstring(str2), debugstr_hstring(str));
     HSTRING_UserFree(&umcb.Flags, &str2);

init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
@@ -718,13 +714,12 @@ static void test_marshal(void)
     ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
     ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
     next = HSTRING_UserUnmarshal(&umcb.Flags, &buffer[1], &str2);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
-    todo_wine_if(str2) ok(str2 != str, "got str2 %p\n", str2);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(str2 != str, "got str2 %p\n", str2);
     order = -1;
     hr = WindowsCompareStringOrdinal(str2, str, &order);
     ok(hr == S_OK, "got hr %#lx\n", hr);
-    todo_wine ok(!order, "got str2 %s != %s\n", debugstr_hstring(str2), debugstr_hstring(str));
+    ok(!order, "got str2 %s != %s\n", debugstr_hstring(str2), debugstr_hstring(str));
     HSTRING_UserFree(&umcb.Flags, &str2);

init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
@@ -739,8 +734,7 @@ static void test_marshal(void)
     ok(!wire->buf.size, "got buf.size %lu\n", wire->buf.size);
     str2 = NULL;
     next = HSTRING_UserUnmarshal(&umcb.Flags, buffer, &str2);
-    if (size == exp_size)
-        todo_wine ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
+    ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
     ok(!str2, "got str2 %p\n", str2);
     HSTRING_UserFree(&umcb.Flags, &str2);

diff --git a/dlls/combase/usrmarshal.c b/dlls/combase/usrmarshal.c
index f09fb5ebe5f..22ab98742cd 100644
--- a/dlls/combase/usrmarshal.c
+++ b/dlls/combase/usrmarshal.c
@@ -1107,7 +1107,35 @@ BYTE * __RPC_USER HSTRING_UserMarshal(ULONG *flags, BYTE *buf, HSTRING *str)
  */
 BYTE * __RPC_USER HSTRING_UserUnmarshal(ULONG *flags, BYTE *buf, HSTRING *str)
 {
-    FIXME("%p, %p, %p: stub\n", flags, buf, str);
+    const struct hstring_wire *wire;
+#ifdef _WIN64
+    static const ULONG prefix = WDT_INPROC64_CALL;
+#else
+    static const ULONG prefix = WDT_INPROC_CALL;
+#endif
+
+    TRACE("%p, %p, %p\n", debugstr_user_flags(flags), buf, str);
+
+    wire = ALIGNED_POINTER(buf, 7);
+    if (wire->context != prefix)
+        RpcRaiseException(RPC_S_INVALID_TAG);
+    if (LOWORD(*flags) == MSHCTX_INPROC)
+    {
+        *str = wire->str;
+        TRACE("str=%s\n", debugstr_hstring(*str));
+        buf = (BYTE *)(&wire->str + 1);
+    }
+    else
+    {
+        UINT32 len;
+
+        if (wire->buf.size % 2)
+            RpcRaiseException(RPC_S_INVALID_BOUND);
+        len = wire->buf.size/sizeof(WCHAR);
+        WindowsCreateString(wire->buf.data, len, str);
+        buf = (BYTE *)&wire->buf.data[len];
+    }
+
     return buf;
 }

@@ -1116,5 +1144,8 @@ BYTE * __RPC_USER HSTRING_UserUnmarshal(ULONG *flags, BYTE *buf, HSTRING *str)
  */
 void __RPC_USER HSTRING_UserFree(ULONG *flags, HSTRING *str)
 {
-    FIXME("%p, %p: stub\n", flags, str);
+    TRACE("%s, %s.\n", debugstr_user_flags(flags), debugstr_hstring(*str));
+
+    if (LOWORD(*flags) == MSHCTX_INPROC)
+        WindowsDeleteString(*str);
 }

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/combase/usrmarshal.c:
> +{
> - ULONG context;
> - union {
> -
>    struct {
> -
>        ULONG size;
> -
>        WCHAR data[];
> -
>    } buf;
> -
>    struct {
>
> +#ifdef _WIN64
> -
>        ULONG pad;
>
> +#endif
> -
>        HSTRING str;
> -
>    };
> - };
>
> +};
> +#pragma pack(pop)

There should a simpler way to define this. Padding I suspect is due to pointer field alignment on 64-bit. Does it not properly right away?

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/combase/usrmarshal.c:
> -
>        WCHAR data[];
> -
>    } buf;
> -
>    struct {
>
> +#ifdef _WIN64
> -
>        ULONG pad;
>
> +#endif
> -
>        HSTRING str;
> -
>    };
> - };
>
> +};
> +#pragma pack(pop)
> -
>
> +/******************************************************************************
> -
> -
>       HSTRING_UserSize (combase.@)
> - */
>
> +ULONG __RPC_USER HSTRING_UserSize(ULONG *flags, ULONG start, HSTRING *str)

It's not a 'start', it's current buffer 'size', where you're appending HSTRING argument.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/combase/usrmarshal.c:
> - struct hstring_wire *wire;
> -
> - TRACE("%s, %p, %s.\n", debugstr_user_flags(flags), buf, debugstr_hstring(*str));
> -
> - if (LOWORD(*flags) == MSHCTX_DIFFERENTMACHINE)
> - {
> -
>    FIXME("MSHCTX_DIFFERENTMACHINE is not supported yet.\n");
> -
>    RpcRaiseException(RPC_S_INVALID_TAG);
> - }
> -
> - wire = ALIGNED_POINTER(buf, 7);
>
> +#ifdef _WIN64
> - wire->context = WDT_INPROC64_CALL;
>
> +#else
> - wire->context = WDT_INPROC_CALL;
>
> +#endif

Existing code uses compile time sizeof() checks instead of this. Maybe it's better to follow here too.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/combase/tests/string.c:
> -
>        HSTRING str;
> -
>    };
> - };
>
> +};
> +#pragma pack(pop)
> -
>
> +static void test_marshal(void)
> +{
> - static const WCHAR str_buf[] = L"marshal_test1";
> - static const ULONG str_len = ARRAY_SIZE(str_buf) - 1;
> - static const ULONG str_bytes = str_len * sizeof(WCHAR);
>
> +#ifdef _WIN64
> - const ULONG exp_prefix = WDT_INPROC64_CALL;
>
> +#else
> - const ULONG exp_prefix = WDT_INPROC_CALL;
>
> +#endif

This value is called context everywhere else.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/combase/tests/string.c:
> - ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
> - ok(!str2, "got str2 %p\n", str2);
> - HSTRING_UserFree(&umcb.Flags, &str2);
> -
> - /* Out of process marshaling */
> - init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
> - size = HSTRING_UserSize(&umcb.Flags, 0, &str);
> - exp_size = offsetof(struct hstring_wire, buf.data[str_len]);
> - ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
> - memset(buffer, 0, 80);
> - init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
> - next = HSTRING_UserMarshal(&umcb.Flags, buffer, &str);
> - ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
> - wire = (struct hstring_wire *)buffer;
> - ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
> - ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));

You'll need to check context field here too.

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/combase/tests/string.c:
> - init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
> - size = HSTRING_UserSize(&umcb.Flags, 1, &str);
> - exp_size = ALIGNED_LENGTH(1, 7) + offsetof(struct hstring_wire, buf.data[str_len]);
> - ok(size == exp_size, "got size %lu != %lu\n", size, exp_size);
> - memset(buffer, 0, 80);
> - init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
> - next = HSTRING_UserMarshal(&umcb.Flags, &buffer[1], &str);
> - ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
> - wire = ALIGNED_POINTER(&buffer[1], 7);
> - ok(wire->buf.size == str_bytes, "got buf.size %lu != %lu\n", wire->buf.size, str_bytes);
> - ok(!memcmp(wire->buf.data, str_buf, str_bytes), "got buf.data %s\n", debugstr_wn(wire->buf.data, str_bytes));
> - next = HSTRING_UserUnmarshal(&umcb.Flags, &buffer[1], &str2);
> - ok(next == &buffer[size], "got next %p != %p\n", next, &buffer[size]);
> - ok(str2 != str, "got str2 %p\n", str2);
> - order = -1;
> - hr = WindowsCompareStringOrdinal(str2, str, &order);

Could we call that "result" instead? What does 'order' mean in this context?

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/combase/tests/string.c:
>  test_compare();
>  test_trim();
>  test_hstring_struct();
> - test_marshal();

Traditionally that would go to usrmarshal.c, but we are not going to have more user types in combase. So it's fine.