[PATCH v2] combase: Reorder hstring_private elements.

[Bernhard Kölbl]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=51017

Debugging sessions and the official WinRT SDK show, that the
contained string is aligned at the end and the reference flag
at the beginning of the hstring_private struct.

Signed-off-by: Bernhard Kölbl 
---
v2: I was mistaken about no reference counting being used.
---
 dlls/combase/string.c | 85 +++++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 35 deletions(-)

diff --git a/dlls/combase/string.c b/dlls/combase/string.c
index 2092e4360a3..32ca89d53a0 100644
--- a/dlls/combase/string.c
+++ b/dlls/combase/string.c
@@ -26,12 +26,14 @@

WINE_DEFAULT_DEBUG_CHANNEL(winstring);

+#define HSTRING_REFERENCE_FLAG 1
+
 struct hstring_private
 {
-    LPWSTR buffer;
+    UINT32 flags;
     UINT32 length;
-    BOOL   reference;
-    LONG   refcount;
+    LONG refcount;
+    LPWSTR ptr;
 };

static const WCHAR empty[1];
@@ -40,30 +42,30 @@ C_ASSERT(sizeof(struct hstring_private) <= sizeof(HSTRING_HEADER));

static inline struct hstring_private *impl_from_HSTRING(HSTRING string)
 {
-   return (struct hstring_private *)string;
+    return (struct hstring_private *)string;
 }

static inline struct hstring_private *impl_from_HSTRING_HEADER(HSTRING_HEADER *header)
 {
-   return (struct hstring_private *)header;
+    return (struct hstring_private *)header;
 }

static inline struct hstring_private *impl_from_HSTRING_BUFFER(HSTRING_BUFFER buffer)
 {
-   return (struct hstring_private *)buffer;
+    return CONTAINING_RECORD(buffer, struct hstring_private, ptr);
 }

static BOOL alloc_string(UINT32 len, HSTRING *out)
 {
     struct hstring_private *priv;
-    priv = HeapAlloc(GetProcessHeap(), 0, sizeof(*priv) + (len + 1) * sizeof(*priv->buffer));
+    priv = HeapAlloc(GetProcessHeap(), 0, sizeof(*priv));
     if (!priv)
         return FALSE;
-    priv->buffer = (LPWSTR)(priv + 1);
+    priv->ptr = (LPWSTR)HeapAlloc(GetProcessHeap(), 0, (len + 1) * sizeof(*priv->ptr));
     priv->length = len;
-    priv->reference = FALSE;
+    priv->flags = 0;
     priv->refcount = 1;
-    priv->buffer[len] = '\0';
+    priv->ptr[len] = '\0';
     *out = (HSTRING)priv;
     return TRUE;
 }
@@ -90,7 +92,7 @@ HRESULT WINAPI WindowsCreateString(LPCWSTR ptr, UINT32 len,
     if (!alloc_string(len, out))
         return E_OUTOFMEMORY;
     priv = impl_from_HSTRING(*out);
-    memcpy(priv->buffer, ptr, len * sizeof(*priv->buffer));
+    memcpy(priv->ptr, ptr, len * sizeof(*priv->ptr));
     return S_OK;
 }

@@ -115,10 +117,10 @@ HRESULT WINAPI WindowsCreateStringReference(LPCWSTR ptr, UINT32 len,
     }
     if (ptr == NULL)
         return E_POINTER;
-    priv->buffer = (LPWSTR)ptr;
+    priv->ptr = (LPWSTR)ptr;
     priv->length = len;
-    priv->reference = TRUE;
-    *out = (HSTRING)header;
+    priv->flags = HSTRING_REFERENCE_FLAG;
+    *out = (HSTRING)priv;
     return S_OK;
 }

@@ -127,16 +129,21 @@ HRESULT WINAPI WindowsCreateStringReference(LPCWSTR ptr, UINT32 len,
  */
 HRESULT WINAPI WindowsDeleteString(HSTRING str)
 {
-    struct hstring_private *priv = impl_from_HSTRING(str);
+    struct hstring_private *priv;

TRACE("(%p)\n", str);

if (str == NULL)
         return S_OK;
-    if (priv->reference)
+
+    priv = impl_from_HSTRING(str);
+
+    if(priv->flags == 0)
         return S_OK;
+
     if (InterlockedDecrement(&priv->refcount) == 0)
         HeapFree(GetProcessHeap(), 0, priv);
+
     return S_OK;
 }

@@ -156,8 +163,8 @@ HRESULT WINAPI WindowsDuplicateString(HSTRING str, HSTRING *out)
         *out = NULL;
         return S_OK;
     }
-    if (priv->reference)
-        return WindowsCreateString(priv->buffer, priv->length, out);
+    if (priv->flags == HSTRING_REFERENCE_FLAG)
+        return WindowsCreateString(priv->ptr, priv->length, out);
     InterlockedIncrement(&priv->refcount);
     *out = str;
     return S_OK;
@@ -186,8 +193,8 @@ HRESULT WINAPI WindowsPreallocateStringBuffer(UINT32 len, WCHAR **outptr,
     if (!alloc_string(len, &str))
         return E_OUTOFMEMORY;
     priv = impl_from_HSTRING(str);
-    *outptr = priv->buffer;
-    *out = (HSTRING_BUFFER)str;
+    *outptr = priv->ptr;
+    *out = (HSTRING_BUFFER)&priv->ptr;
     return S_OK;
 }

@@ -196,9 +203,17 @@ HRESULT WINAPI WindowsPreallocateStringBuffer(UINT32 len, WCHAR **outptr,
  */
 HRESULT WINAPI WindowsDeleteStringBuffer(HSTRING_BUFFER buf)
 {
+
+    struct hstring_private *priv;
+
     TRACE("(%p)\n", buf);

-    return WindowsDeleteString((HSTRING)buf);
+    if(buf)
+        priv = impl_from_HSTRING_BUFFER(buf);
+    else
+        priv = NULL;
+
+    return WindowsDeleteString((HSTRING)priv);
 }

/***********************************************************************
@@ -217,9 +232,9 @@ HRESULT WINAPI WindowsPromoteStringBuffer(HSTRING_BUFFER buf, HSTRING *out)
         *out = NULL;
         return S_OK;
     }
-    if (priv->buffer[priv->length] != 0 || priv->reference || priv->refcount != 1)
+    if (priv->ptr[priv->length] != 0 || priv->flags == HSTRING_REFERENCE_FLAG || priv->refcount != 1)
         return E_INVALIDARG;
-    *out = (HSTRING)buf;
+    *out = (HSTRING)priv;
     return S_OK;
 }

@@ -254,7 +269,7 @@ LPCWSTR WINAPI WindowsGetStringRawBuffer(HSTRING str, UINT32 *len)
     }
     if (len)
         *len = priv->length;
-    return priv->buffer;
+    return priv->ptr;
 }

/***********************************************************************
@@ -276,7 +291,7 @@ HRESULT WINAPI WindowsStringHasEmbeddedNull(HSTRING str, BOOL *out)
     }
     for (i = 0; i < priv->length; i++)
     {
-        if (priv->buffer[i] == '\0')
+        if (priv->ptr[i] == '\0')
         {
             *out = TRUE;
             return S_OK;
@@ -305,7 +320,7 @@ HRESULT WINAPI WindowsSubstring(HSTRING str, UINT32 start, HSTRING *out)
         *out = NULL;
         return S_OK;
     }
-    return WindowsCreateString(&priv->buffer[start], len - start, out);
+    return WindowsCreateString(&priv->ptr[start], len - start, out);
 }

/***********************************************************************
@@ -327,7 +342,7 @@ HRESULT WINAPI WindowsSubstringWithSpecifiedLength(HSTRING str, UINT32 start, UI
         *out = NULL;
         return S_OK;
     }
-    return WindowsCreateString(&priv->buffer[start], len, out);
+    return WindowsCreateString(&priv->ptr[start], len, out);
 }

/***********************************************************************
@@ -355,8 +370,8 @@ HRESULT WINAPI WindowsConcatString(HSTRING str1, HSTRING str2, HSTRING *out)
     if (!alloc_string(priv1->length + priv2->length, out))
         return E_OUTOFMEMORY;
     priv = impl_from_HSTRING(*out);
-    memcpy(priv->buffer, priv1->buffer, priv1->length * sizeof(*priv1->buffer));
-    memcpy(priv->buffer + priv1->length, priv2->buffer, priv2->length * sizeof(*priv2->buffer));
+    memcpy(priv->ptr, priv1->ptr, priv1->length * sizeof(*priv1->ptr));
+    memcpy(priv->ptr + priv1->length, priv2->ptr, priv2->length * sizeof(*priv2->ptr));
     return S_OK;
 }

@@ -395,12 +410,12 @@ HRESULT WINAPI WindowsCompareStringOrdinal(HSTRING str1, HSTRING str2, INT32 *re
     }
     if (str1)
     {
-        buf1 = priv1->buffer;
+        buf1 = priv1->ptr;
         len1 = priv1->length;
     }
     if (str2)
     {
-        buf2 = priv2->buffer;
+        buf2 = priv2->ptr;
         len2 = priv2->length;
     }
     *res = CompareStringOrdinal(buf1, len1, buf2, len2, FALSE) - CSTR_EQUAL;
@@ -427,10 +442,10 @@ HRESULT WINAPI WindowsTrimStringStart(HSTRING str1, HSTRING str2, HSTRING *out)
     }
     for (start = 0; start < priv1->length; start++)
     {
-        if (!wmemchr(priv2->buffer, priv1->buffer[start], priv2->length))
+        if (!wmemchr(priv2->ptr, priv1->ptr[start], priv2->length))
             break;
     }
-    return start ? WindowsCreateString(&priv1->buffer[start], priv1->length - start, out) :
+    return start ? WindowsCreateString(&priv1->ptr[start], priv1->length - start, out) :
                    WindowsDuplicateString(str1, out);
 }

@@ -454,9 +469,9 @@ HRESULT WINAPI WindowsTrimStringEnd(HSTRING str1, HSTRING str2, HSTRING *out)
     }
     for (len = priv1->length; len > 0; len--)
     {
-        if (!wmemchr(priv2->buffer, priv1->buffer[len - 1], priv2->length))
+        if (!wmemchr(priv2->ptr, priv1->ptr[len - 1], priv2->length))
             break;
     }
-    return (len < priv1->length) ? WindowsCreateString(priv1->buffer, len, out) :
+    return (len < priv1->length) ? WindowsCreateString(priv1->ptr, len, out) :
                                    WindowsDuplicateString(str1, out);
 }

[Nikolay Sivov]

On 12/24/21 01:48, Bernhard Kölbl wrote:
> +#define HSTRING_REFERENCE_FLAG 1
> - struct hstring_private
> {
> - LPWSTR buffer;
> - UINT32 flags;
>  UINT32 length;
> - BOOL   reference;
> - LONG   refcount;
> - LONG refcount;
> - LPWSTR ptr;
> };

From what I can tell this still doesn't match native layout. It also 
might be slightly different for references.
> static BOOL alloc_string(UINT32 len, HSTRING *out)
>   {
>       struct hstring_private *priv;
> - priv = HeapAlloc(GetProcessHeap(), 0, sizeof(*priv) + (len + 1) * sizeof(*priv->buffer));
> - priv = HeapAlloc(GetProcessHeap(), 0, sizeof(*priv));
>  if (!priv)
>      return FALSE;
> - priv->buffer = (LPWSTR)(priv + 1);
> - priv->ptr = (LPWSTR)HeapAlloc(GetProcessHeap(), 0, (len + 1) * sizeof(*priv->ptr));

I don't see this behaviour on Windows.

But anyway, do you have any idea why application would care? Is it some 
statically linked code depending on it, or some native module bundled or 
installed separately?

If the layout is stable across Windows releases, we'll need tests for it.