[PATCH 0/2] MR6900: oleaut32: Make OleCreateFontIndirect return error if font name is missing

[Fabian Maurer (＠DarkShadow44)]

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

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

[PATCH 1/2] oleaut32: Make OleCreateFontIndirect return error if font name is missing

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57457
---
 dlls/oleaut32/olefont.c       |  3 +++
 dlls/oleaut32/tests/olefont.c | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/dlls/oleaut32/olefont.c b/dlls/oleaut32/olefont.c
index aea8b596498..78361e5f9fc 100644
--- a/dlls/oleaut32/olefont.c
+++ b/dlls/oleaut32/olefont.c
@@ -323,6 +323,9 @@ HRESULT WINAPI OleCreateFontIndirect(
 
   *ppvObj = 0;
 
+  if (lpFontDesc && !lpFontDesc->lpstrName)
+      return CTL_E_INVALIDPROPERTYVALUE;
+
   if (!lpFontDesc) {
     static WCHAR fname[] = L"System";
 
diff --git a/dlls/oleaut32/tests/olefont.c b/dlls/oleaut32/tests/olefont.c
index a8736cd0172..3fe546f7b47 100644
--- a/dlls/oleaut32/tests/olefont.c
+++ b/dlls/oleaut32/tests/olefont.c
@@ -1224,6 +1224,7 @@ static void test_OleCreateFontIndirect(void)
     IUnknown *unk, *unk2;
     IFont *font;
     HRESULT hr;
+    WCHAR str_empty[] = {0};
 
     fontdesc.cbSizeofstruct = sizeof(fontdesc);
     fontdesc.lpstrName = arial_font;
@@ -1254,6 +1255,22 @@ static void test_OleCreateFontIndirect(void)
     EXPECT_HR(hr, S_OK);
     IFont_Release(font);
 
+    /* Test NULL name */
+    fontdesc.cbSizeofstruct = sizeof(fontdesc);
+    fontdesc.lpstrName = NULL;
+    font = (IFont*)0xdeadbeef;
+    hr = pOleCreateFontIndirect(&fontdesc, &IID_IFont, (void**)&font);
+    EXPECT_HR(hr, CTL_E_INVALIDPROPERTYVALUE);
+    ok(font == 0, "Got %p\n", font);
+
+    /* Test empty Name */
+    fontdesc.lpstrName = str_empty;
+    font = 0;
+    hr = pOleCreateFontIndirect(&fontdesc, &IID_IFont, (void**)&font);
+    EXPECT_HR(hr, S_OK);
+    ok(font != 0, "Got NULL font\n");
+    IFont_Release(font);
+
     hr = OleInitialize(NULL);
     ok(hr == S_OK, "got 0x%08lx\n", hr);
 
-- 
GitLab


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

[PATCH 2/2] oleaut32: Remove a new unneeded null checks

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

Now that we check for null in the constructor, the name
can't be null anymore
---
 dlls/oleaut32/olefont.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/dlls/oleaut32/olefont.c b/dlls/oleaut32/olefont.c
index 78361e5f9fc..6a98067b0bf 100644
--- a/dlls/oleaut32/olefont.c
+++ b/dlls/oleaut32/olefont.c
@@ -637,10 +637,7 @@ static HRESULT WINAPI OLEFontImpl_get_Name(
 
   realize_font(this);
 
-  if (this->description.lpstrName!=0)
-    *pname = SysAllocString(this->description.lpstrName);
-  else
-    *pname = 0;
+  *pname = SysAllocString(this->description.lpstrName);
 
   return S_OK;
 }
@@ -1673,11 +1670,8 @@ static HRESULT WINAPI OLEFontImpl_Save(
   if (written != sizeof(DWORD)) return E_FAIL;
 
   /* FontName */
-  if (this->description.lpstrName)
-    string_size = WideCharToMultiByte( CP_ACP, 0, this->description.lpstrName,
-                                       lstrlenW(this->description.lpstrName), NULL, 0, NULL, NULL );
-  else
-    string_size = 0;
+  string_size = WideCharToMultiByte( CP_ACP, 0, this->description.lpstrName,
+                                     lstrlenW(this->description.lpstrName), NULL, 0, NULL, NULL );
 
   IStream_Write(pOutStream, &string_size, sizeof(BYTE), &written);
   if (written != sizeof(BYTE)) return E_FAIL;
@@ -1720,10 +1714,9 @@ static HRESULT WINAPI OLEFontImpl_GetSizeMax(
   pcbSize->u.LowPart += sizeof(DWORD); /* Size */
   pcbSize->u.LowPart += sizeof(BYTE);  /* StrLength */
 
-  if (this->description.lpstrName!=0)
-      pcbSize->u.LowPart += WideCharToMultiByte( CP_ACP, 0, this->description.lpstrName,
-                                                 lstrlenW(this->description.lpstrName),
-                                                 NULL, 0, NULL, NULL );
+  pcbSize->u.LowPart += WideCharToMultiByte( CP_ACP, 0, this->description.lpstrName,
+                                             lstrlenW(this->description.lpstrName),
+                                             NULL, 0, NULL, NULL );
 
   return S_OK;
 }
-- 
GitLab

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

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/oleaut32/olefont.c:
> *ppvObj = 0;
>
> +  if (lpFontDesc && !lpFontDesc->lpstrName)
> +      return CTL_E_INVALIDPROPERTYVALUE;
> +
>    if (!lpFontDesc) {
>      static WCHAR fname[] = L"System";

I'd put that as "else" branch of the existing "if".

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6900#note_88891

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/oleaut32/tests/olefont.c:
> EXPECT_HR(hr, S_OK);
>      IFont_Release(font);
>
> +    /* Test NULL name */
> +    fontdesc.cbSizeofstruct = sizeof(fontdesc);
> +    fontdesc.lpstrName = NULL;
> +    font = (IFont*)0xdeadbeef;
> +    hr = pOleCreateFontIndirect(&fontdesc, &IID_IFont, (void**)&font);
> +    EXPECT_HR(hr, CTL_E_INVALIDPROPERTYVALUE);
> +    ok(font == 0, "Got %p\n", font);
> +
> +    /* Test empty Name */
> +    fontdesc.lpstrName = str_empty;
> +    font = 0;

Maybe NULL?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6900#note_88890