Attachment 358104 Details for Bug 192805 – Patch

[patch] Patch

bug-192805-20181227172404.patch (text/plain), 2.86 KB, created by Fujii Hironori on 2018-12-27 00:24:05 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Fujii Hironori

Created: 2018-12-27 00:24:05 PST

Size: 2.86 KB

patch

obsolete

>Subversion Revision: 239555
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a238b4f785212d81a435b4efa9938518e85d4320..3eacaf6a12d4b0cc25df58f1664715fda9514955 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-12-26  Fujii Hironori  <Hironori.Fujii@sony.com>
>+
>+        [Win] Fix the wrong return value checking of ScriptItemize in UniscribeController::itemizeShapeAndPlace
>+        https://bugs.webkit.org/show_bug.cgi?id=192805
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        UniscribeController::itemizeShapeAndPlace has a error check
>+        'FAILED(rc)' for the return value of ScriptItemize. However, it
>+        didn't actually check the return value.
>+
>+        The equal comparison '==' has higher operator precedence than the
>+        assignment '='.
>+
>+        1. rc = ::ScriptItemize(...) == E_OUTOFMEMORY
>+        2. rc = (::ScriptItemize(...) == E_OUTOFMEMORY)
>+        3. (rc = ::ScriptItemize(...)) == E_OUTOFMEMORY
>+
>+        Thus, the expression #1 is same with #2, not #3. As the result,
>+        'FAILED(rc)' never be true.
>+
>+        Fortunately, as far as I understand the code, there is not an
>+        actual problem because ScriptItemize never returns E_INVALIDARG in
>+        the code.
>+
>+        No new tests, no behavior changes.
>+
>+        * platform/graphics/win/UniscribeController.cpp:
>+        (WebCore::UniscribeController::itemizeShapeAndPlace): Use
>+        parentheses to assign 'rc' the return code. Replaced the useless
>+        error check with an assertion ensuring 'SUCCEEDED(rc)'.
>+
> 2018-12-26  Jim Mason  <jmason@ibinx.com>
> 
>         [FreeType] Restore conditional compilation logic for recent HarfBuzz refactoring
>diff --git a/Source/WebCore/platform/graphics/win/UniscribeController.cpp b/Source/WebCore/platform/graphics/win/UniscribeController.cpp
>index 440a0a1e1cdd211044802417f2d9f1c3c9d38aec..938c402882fcb0f763f958b1761890b247226c7c 100644
>--- a/Source/WebCore/platform/graphics/win/UniscribeController.cpp
>+++ b/Source/WebCore/platform/graphics/win/UniscribeController.cpp
>@@ -196,14 +196,11 @@ void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned length,
>     m_items.resize(6);
>     int numItems = 0;
>     HRESULT rc = S_OK;
>-    while (rc = ::ScriptItemize(cp, length, m_items.size() - 1, &m_control, &m_state, m_items.data(), &numItems) == E_OUTOFMEMORY) {
>+    while ((rc = ::ScriptItemize(cp, length, m_items.size() - 1, &m_control, &m_state, m_items.data(), &numItems)) == E_OUTOFMEMORY) {
>         m_items.resize(m_items.size() * 2);
>         resetControlAndState();
>     }
>-    if (FAILED(rc)) {
>-        WTFLogAlways("UniscribeController::itemizeShapeAndPlace: ScriptItemize failed, rc=%lx", rc);
>-        return;
>-    }
>+    ASSERT_UNUSED(rc, SUCCEEDED(rc)); // ScriptItemize must not return E_INVALIDARG in this code.
>     m_items.resize(numItems + 1);
> 
>     if (m_run.rtl()) {

Actions: View | Formatted Diff | Diff

Attachments on bug 192805: 357542 | 358103 | 358104