[PATCH 0/1] MR1325: oleaut32: Properly store/load BSTR:s as multi-byte strings.

[eric pouech (＠epo)]

Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=52873

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com>

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

[PATCH 1/1] oleaut32: Properly store/load BSTR:s as multi-byte strings.

[Eric Pouech]

From: Eric Pouech <eric.pouech(a)gmail.com>

Wine-Bugs: https://bugs.winehq.org/show_bug.cgi?id=52873

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com>
---
 dlls/oleaut32/typelib.c | 47 +++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c
index a7bbb273856..cc7dfe50d32 100644
--- a/dlls/oleaut32/typelib.c
+++ b/dlls/oleaut32/typelib.c
@@ -2246,22 +2246,26 @@ static void MSFT_ReadValue( VARIANT * pVar, int offset, TLBContext *pcx )
         case VT_FILETIME :
             size=8;break;
             /* pointer types with known behaviour */
-        case VT_BSTR    :{
-            char * ptr;
+        case VT_BSTR    :
             MSFT_ReadLEDWords(&size, sizeof(INT), pcx, DO_NOT_SEEK );
             if(size == -1){
                 V_BSTR(pVar) = NULL;
             }else{
+                char * ptr;
+                DWORD len;
                 ptr = heap_alloc_zero(size);
                 MSFT_Read(ptr, size, pcx, DO_NOT_SEEK);
-                V_BSTR(pVar)=SysAllocStringLen(NULL,size);
-                /* FIXME: do we need a AtoW conversion here? */
-                V_UNION(pVar, bstrVal[size])='\0';
-                while(size--) V_UNION(pVar, bstrVal[size])=ptr[size];
+                len = MultiByteToWideChar(CP_ACP, 0, ptr, size, NULL, 0);
+                V_BSTR(pVar) = SysAllocStringLen(NULL, len);
+                if (V_BSTR(pVar))
+                {
+                    MultiByteToWideChar(CP_ACP, 0, ptr, size, V_BSTR(pVar), len);
+                    V_UNION(pVar, bstrVal[len]) = L'\0';
+                }
                 heap_free(ptr);
+                size = 0;
             }
-	}
-	size=-4; break;
+            break;
     /* FIXME: this will not work AT ALL when the variant contains a pointer */
         case VT_DISPATCH :
         case VT_VARIANT :
@@ -5734,23 +5738,12 @@ static HRESULT TLB_CopyElemDesc( const ELEMDESC *src, ELEMDESC *dest, char **buf
     return S_OK;
 }
 
-static HRESULT TLB_SanitizeBSTR(BSTR str)
-{
-    UINT len = SysStringLen(str), i;
-    for (i = 0; i < len; ++i)
-        if (str[i] > 0x7f)
-            str[i] = '?';
-    return S_OK;
-}
-
 static HRESULT TLB_SanitizeVariant(VARIANT *var)
 {
     if (V_VT(var) == VT_INT)
         return VariantChangeType(var, var, 0, VT_I4);
     else if (V_VT(var) == VT_UINT)
         return VariantChangeType(var, var, 0, VT_UI4);
-    else if (V_VT(var) == VT_BSTR)
-        return TLB_SanitizeBSTR(V_BSTR(var));
 
     return S_OK;
 }
@@ -9421,7 +9414,8 @@ static DWORD WMSFT_encode_variant(VARIANT *value, WMSFT_TLBFile *file)
     }
 
     case VT_BSTR: {
-        int i, len = (6+SysStringLen(V_BSTR(&v))+3) & ~0x3;
+        int mblen = WideCharToMultiByte(CP_ACP, 0, V_BSTR(&v), SysStringLen(V_BSTR(&v)), NULL, 0, NULL, NULL);
+        int len = (6 + mblen + 3) & ~0x3;
         char *data;
 
         if(file->custdata_seg.data){
@@ -9434,16 +9428,9 @@ static DWORD WMSFT_encode_variant(VARIANT *value, WMSFT_TLBFile *file)
         }
 
         *((unsigned short *)data) = V_VT(value);
-        *((unsigned int *)(data+2)) = SysStringLen(V_BSTR(&v));
-        for(i=0; i<SysStringLen(V_BSTR(&v)); i++) {
-            if(V_BSTR(&v)[i] <= 0x7f)
-                data[i+6] = V_BSTR(&v)[i];
-            else
-                data[i+6] = '?';
-        }
-        WideCharToMultiByte(CP_ACP, 0, V_BSTR(&v), SysStringLen(V_BSTR(&v)), &data[6], len-6, NULL, NULL);
-        for(i=6+SysStringLen(V_BSTR(&v)); i<len; i++)
-            data[i] = 0x57;
+        *((unsigned int *)(data+2)) = mblen;
+        WideCharToMultiByte(CP_ACP, 0, V_BSTR(&v), SysStringLen(V_BSTR(&v)), &data[6], mblen, NULL, NULL);
+        memset(&data[6 + mblen], 0x57, len - (6 + mblen));
 
         /* TODO: Check if the encoded data is already present in custdata_seg */
 
-- 
GitLab

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

[PATCH 0/1] MR1325: oleaut32: Properly store/load BSTR:s as multi-byte strings. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1325

[Huw Davies (＠huw)]

Actually, the conversion should probably use the default codepage for the lcid that's stored in the typelib's header.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1325#note_14036

[eric pouech (＠epo)]

Hi Huw,

did you actually test it?
I tried this https://testbot.winehq.org/JobDetails.pl?Key=126114, which doesn't suggest that. May be I got the test wrong?

Note:
- I also tried to check if we could get rid of the direct unicode comparison in compare_wstr, as it looked better to me to have only a single test strategy (unicode => multibyte => unicode)
- but if we need to go back to default (ansi or oem) cp from lcid, then we havea detect locales without codepages (hence requiring the two tests above)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1325#note_15597

[eric pouech (＠epo)]

@huw ping?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1325#note_16912

[PATCH 0/1] MR1325: oleaut32: Properly store/load BSTR:s as multi-byte strings. - closed

[eric pouech (＠epo)]

This merge request was closed by eric pouech.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1325

[eric pouech (＠epo)]

Alexandre committed something similar in 680d17b453ad651ecb12be5d12a7e6baf84a4de1.
Despite Huw comments on this patch.
Closing.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1325#note_19615

[Alexandre Julliard (＠julliard)]

> Alexandre committed something similar in 680d17b453ad651ecb12be5d12a7e6baf84a4de1. Despite Huw comments on this patch. Closing.

Ah sorry, I forgot about this one.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1325#note_19629