[Win] Resolve various static analyzer warnings in WebCore.
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Oct 2014 18:32:12 +0000 (18:32 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Oct 2014 18:32:12 +0000 (18:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=137526

Reviewed by Dean Jackson.

A series of small changes to resolve various issues found by the MSVC static analyzer.

* inspector/NetworkResourcesData.cpp:
(WebCore::NetworkResourcesData::clear): Add assertion that it->value should never be null.
* page/SessionIDHash.h:
(WTF::HashTraits<WebCore::SessionID>::constructDeletedValue): Add explicit cast to avoid
compiler warning.
(WTF::HashTraits<WebCore::SessionID>::isDeletedValue): Ditto.
* page/win/FrameCGWin.cpp:
(WebCore::imageFromRect): Resolve static analyzer warnings by initializing bits, and
checking the return value of ::CreateDIBSection, which return nullptr on error.
* platform/graphics/ca/win/PlatformCALayerWin.cpp:
(printLayer): Use correct MSVC format specifier for size_t.
* platform/graphics/win/FontCacheWin.cpp:
(WebCore::getLinkedFonts): Handle possibility that a font link key does not exist.
(WebCore::FontCache::systemFallbackForCharacters): Handle error case when a valid code page
does not exist for a given character.
* platform/graphics/win/SimpleFontDataWin.cpp:
(WebCore::SimpleFontData::containsCharacters): Handle error cases for mapping to the CP_ACP code page,
and related failures when attempting to access the contents of a given code page.
* platform/graphics/win/UniscribeController.cpp:
(WebCore::UniscribeController::itemizeShapeAndPlace): Handle possible failure in the
ScriptItemize API call.
(WebCore::UniscribeController::shapeAndPlaceItem): Ditto for ScriptXtoCP API call.
* platform/win/BString.h: Use consistent SAL annotations for our typedeof of BSTR as in
the system header.
* platform/win/COMPtr.h: Ditto for HRESULT.
* platform/win/DragImageCGWin.cpp:
(WebCore::allocImage): Handle case of failing CreateDIBSection API call.
* platform/win/PopupMenuWin.cpp:
(WebCore::PopupMenuWin::show): Handle case of failing SystemParamtersInfo API call.
(WebCore::PopupMenuWin::wndProc): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174465 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/WebCore/ChangeLog
Source/WebCore/inspector/NetworkResourcesData.cpp
Source/WebCore/page/SessionIDHash.h
Source/WebCore/page/win/FrameCGWin.cpp
Source/WebCore/platform/graphics/ca/win/PlatformCALayerWin.cpp
Source/WebCore/platform/graphics/win/FontCacheWin.cpp
Source/WebCore/platform/graphics/win/SimpleFontDataWin.cpp
Source/WebCore/platform/graphics/win/UniscribeController.cpp
Source/WebCore/platform/win/BString.h
Source/WebCore/platform/win/COMPtr.h
Source/WebCore/platform/win/DragImageCGWin.cpp
Source/WebCore/platform/win/PopupMenuWin.cpp

index a15b78b..b5ba587 100644 (file)
@@ -1,3 +1,43 @@
+2014-10-08  Brent Fulgham  <bfulgham@apple.com>
+
+        [Win] Resolve various static analyzer warnings in WebCore.
+        https://bugs.webkit.org/show_bug.cgi?id=137526
+
+        Reviewed by Dean Jackson.
+
+        A series of small changes to resolve various issues found by the MSVC static analyzer.
+
+        * inspector/NetworkResourcesData.cpp:
+        (WebCore::NetworkResourcesData::clear): Add assertion that it->value should never be null.
+        * page/SessionIDHash.h:
+        (WTF::HashTraits<WebCore::SessionID>::constructDeletedValue): Add explicit cast to avoid
+        compiler warning.
+        (WTF::HashTraits<WebCore::SessionID>::isDeletedValue): Ditto.
+        * page/win/FrameCGWin.cpp:
+        (WebCore::imageFromRect): Resolve static analyzer warnings by initializing bits, and
+        checking the return value of ::CreateDIBSection, which return nullptr on error.
+        * platform/graphics/ca/win/PlatformCALayerWin.cpp:
+        (printLayer): Use correct MSVC format specifier for size_t.
+        * platform/graphics/win/FontCacheWin.cpp:
+        (WebCore::getLinkedFonts): Handle possibility that a font link key does not exist.
+        (WebCore::FontCache::systemFallbackForCharacters): Handle error case when a valid code page
+        does not exist for a given character.
+        * platform/graphics/win/SimpleFontDataWin.cpp:
+        (WebCore::SimpleFontData::containsCharacters): Handle error cases for mapping to the CP_ACP code page,
+        and related failures when attempting to access the contents of a given code page.
+        * platform/graphics/win/UniscribeController.cpp:
+        (WebCore::UniscribeController::itemizeShapeAndPlace): Handle possible failure in the
+        ScriptItemize API call.
+        (WebCore::UniscribeController::shapeAndPlaceItem): Ditto for ScriptXtoCP API call.
+        * platform/win/BString.h: Use consistent SAL annotations for our typedeof of BSTR as in
+        the system header.
+        * platform/win/COMPtr.h: Ditto for HRESULT.
+        * platform/win/DragImageCGWin.cpp:
+        (WebCore::allocImage): Handle case of failing CreateDIBSection API call.
+        * platform/win/PopupMenuWin.cpp:
+        (WebCore::PopupMenuWin::show): Handle case of failing SystemParamtersInfo API call.
+        (WebCore::PopupMenuWin::wndProc): Ditto.
+
 2014-10-07  Jer Noble  <jer.noble@apple.com>
 
         [EME][Mac] Update CDMSessionMediaSourceAVFObjC to match new API provided by AVStreamSession
index 5b8349d..ded654b 100644 (file)
@@ -338,6 +338,7 @@ void NetworkResourcesData::clear(const String& preservedLoaderId)
     ResourceDataMap::iterator end = m_requestIdToResourceDataMap.end();
     for (it = m_requestIdToResourceDataMap.begin(); it != end; ++it) {
         ResourceData* resourceData = it->value;
+        ASSERT(resourceData);
         if (!preservedLoaderId.isNull() && resourceData->loaderId() == preservedLoaderId)
             preservedMap.set(it->key, it->value);
         else
index da984ab..abbfd1a 100644 (file)
 
 namespace WTF {
 
-// The empty value is emptySessionID(), the deleted value is (-1)
+// The empty value is emptySessionID(), the deleted value is (-2)
 struct SessionIDHash {
     static unsigned hash(const WebCore::SessionID& p) { return (unsigned)p.sessionID(); }
     static bool equal(const WebCore::SessionID& a, const WebCore::SessionID& b) { return a == b; }
     static const bool safeToCompareToEmptyOrDeleted = true;
 };
 template<> struct HashTraits<WebCore::SessionID> : GenericHashTraits<WebCore::SessionID> {
+    static const uint64_t deletedValueIdentifier = 0xFFFFFFFFFFFFFFFE;
     static const bool needsDestruction = false;
     static WebCore::SessionID emptyValue() { return WebCore::SessionID::emptySessionID(); }
 
-    static void constructDeletedValue(WebCore::SessionID& slot) { slot = WebCore::SessionID(-2); }
-    static bool isDeletedValue(const WebCore::SessionID& slot) { return slot == WebCore::SessionID(-2); }
+    static void constructDeletedValue(WebCore::SessionID& slot) { slot = WebCore::SessionID(deletedValueIdentifier); }
+    static bool isDeletedValue(const WebCore::SessionID& slot) { return slot == WebCore::SessionID(deletedValueIdentifier); }
 };
 template<> struct DefaultHash<WebCore::SessionID> {
     typedef SessionIDHash Hash;
index d2cfe46..4c0f921 100644 (file)
@@ -55,13 +55,16 @@ GDIObject<HBITMAP> imageFromRect(const Frame* frame, IntRect& ir)
     PaintBehavior oldPaintBehavior = frame->view()->paintBehavior();
     frame->view()->setPaintBehavior(oldPaintBehavior | PaintBehaviorFlattenCompositingLayers);
 
-    void* bits;
+    void* bits = nullptr;
     auto hdc = adoptGDIObject(::CreateCompatibleDC(0));
     int w = ir.width();
     int h = ir.height();
     BitmapInfo bmp = BitmapInfo::create(IntSize(w, h));
 
     GDIObject<HBITMAP> hbmp = adoptGDIObject(::CreateDIBSection(0, &bmp, DIB_RGB_COLORS, static_cast<void**>(&bits), 0, 0));
+    if (!hbmp)
+        return hbmp;
+
     HGDIOBJ hbmpOld = SelectObject(hdc.get(), hbmp.get());
     CGContextRef context = CGBitmapContextCreate(static_cast<void*>(bits), w, h,
         8, w * sizeof(RGBQUAD), deviceRGBColorSpaceRef(), kCGBitmapByteOrder32Little | kCGImageAlphaPremultipliedFirst);
index fd6c1a7..42b309f 100644 (file)
@@ -669,7 +669,7 @@ static void printLayer(const PlatformCALayer* layer, int indent)
         if (CFGetTypeID(layerContents) == CGImageGetTypeID()) {
             CGImageRef imageContents = static_cast<CGImageRef>(const_cast<void*>(layerContents));
             printIndent(indent + 1);
-            fprintf(stderr, "(contents (image [%d %d]))\n",
+            fprintf(stderr, "(contents (image [%Iu %Iu]))\n",
                 CGImageGetWidth(imageContents), CGImageGetHeight(imageContents));
         }
     }
index 845c77f..e8cafa0 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2008, 2013 Apple Inc.  All rights reserved.
+ * Copyright (C) 2006, 2007, 2008, 2013-2014 Apple Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -35,6 +35,7 @@
 #include <mlang.h>
 #include <windows.h>
 #include <wtf/StdLibExtras.h>
+#include <wtf/text/CString.h>
 #include <wtf/text/StringHash.h>
 #include <wtf/text/StringView.h>
 #include <wtf/win/GDIObject.h>
@@ -97,12 +98,16 @@ static const Vector<String>* getLinkedFonts(String& family)
 
     result = new Vector<String>;
     systemLinkMap.set(family, result);
-    HKEY fontLinkKey;
+    HKEY fontLinkKey = nullptr;
     if (FAILED(RegOpenKeyEx(HKEY_LOCAL_MACHINE, L"Software\\Microsoft\\Windows NT\\CurrentVersion\\FontLink\\SystemLink", 0, KEY_READ, &fontLinkKey)))
         return result;
 
     DWORD linkedFontsBufferSize = 0;
-    RegQueryValueEx(fontLinkKey, family.charactersWithNullTermination().data(), 0, NULL, NULL, &linkedFontsBufferSize);
+    if (::RegQueryValueEx(fontLinkKey, family.charactersWithNullTermination().data(), 0, nullptr, nullptr, &linkedFontsBufferSize) == ERROR_FILE_NOT_FOUND) {
+        WTFLogAlways("The font link key %s does not exist in the registry.", family.utf8().data());
+        return result;
+    }
+
     WCHAR* linkedFonts = reinterpret_cast<WCHAR*>(malloc(linkedFontsBufferSize));
     if (SUCCEEDED(RegQueryValueEx(fontLinkKey, family.charactersWithNullTermination().data(), 0, NULL, reinterpret_cast<BYTE*>(linkedFonts), &linkedFontsBufferSize))) {
         unsigned i = 0;
@@ -200,29 +205,29 @@ PassRefPtr<SimpleFontData> FontCache::systemFallbackForCharacters(const FontDesc
     if (IMLangFontLinkType* langFontLink = getFontLinkInterface()) {
         // Try MLang font linking first.
         DWORD codePages = 0;
-        langFontLink->GetCharCodePages(character, &codePages);
-
-        if (codePages && u_getIntPropertyValue(character, UCHAR_UNIFIED_IDEOGRAPH)) {
-            // The CJK character may belong to multiple code pages. We want to
-            // do font linking against a single one of them, preferring the default
-            // code page for the user's locale.
-            const Vector<DWORD, 4>& CJKCodePageMasks = getCJKCodePageMasks();
-            unsigned numCodePages = CJKCodePageMasks.size();
-            for (unsigned i = 0; i < numCodePages && !hfont; ++i) {
-                hfont = createMLangFont(langFontLink, hdc, CJKCodePageMasks[i]);
-                if (hfont && !(codePages & CJKCodePageMasks[i])) {
-                    // We asked about a code page that is not one of the code pages
-                    // returned by MLang, so the font might not contain the character.
-                    SelectObject(hdc, hfont);
-                    if (!currentFontContainsCharacter(hdc, character)) {
-                        DeleteObject(hfont);
-                        hfont = 0;
+        if (SUCCEEDED(langFontLink->GetCharCodePages(character, &codePages))) {
+            if (codePages && u_getIntPropertyValue(character, UCHAR_UNIFIED_IDEOGRAPH)) {
+                // The CJK character may belong to multiple code pages. We want to
+                // do font linking against a single one of them, preferring the default
+                // code page for the user's locale.
+                const Vector<DWORD, 4>& CJKCodePageMasks = getCJKCodePageMasks();
+                unsigned numCodePages = CJKCodePageMasks.size();
+                for (unsigned i = 0; i < numCodePages && !hfont; ++i) {
+                    hfont = createMLangFont(langFontLink, hdc, CJKCodePageMasks[i]);
+                    if (hfont && !(codePages & CJKCodePageMasks[i])) {
+                        // We asked about a code page that is not one of the code pages
+                        // returned by MLang, so the font might not contain the character.
+                        SelectObject(hdc, hfont);
+                        if (!currentFontContainsCharacter(hdc, character)) {
+                            DeleteObject(hfont);
+                            hfont = 0;
+                        }
+                        SelectObject(hdc, primaryFont);
                     }
-                    SelectObject(hdc, primaryFont);
                 }
-            }
-        } else
-            hfont = createMLangFont(langFontLink, hdc, codePages, character);
+            } else
+                hfont = createMLangFont(langFontLink, hdc, codePages, character);
+        }
     }
 
     // A font returned from MLang is trusted to contain the character.
index 9658456..60e7ee7 100644 (file)
@@ -155,16 +155,23 @@ bool SimpleFontData::containsCharacters(const UChar* characters, int length) con
     HWndDC dc(0);
 
     DWORD acpCodePages;
-    langFontLink->CodePageToCodePages(CP_ACP, &acpCodePages);
+    if (FAILED(langFontLink->CodePageToCodePages(CP_ACP, &acpCodePages))) {
+        WTFLogAlways("SimpleFontData::containsCharacters: Unable to convert to CP_ACP code page.");
+        return false;
+    }
 
     DWORD fontCodePages;
-    langFontLink->GetFontCodePages(dc, m_platformData.hfont(), &fontCodePages);
+    if (FAILED(langFontLink->GetFontCodePages(dc, m_platformData.hfont(), &fontCodePages))) {
+        WTFLogAlways("SimpleFontData::containsCharacters: Unable to find matching code page for specified font.");
+        return false;
+    }
 
-    DWORD actualCodePages;
-    long numCharactersProcessed;
+    DWORD actualCodePages = 0;
+    long numCharactersProcessed = 0;
     long offset = 0;
     while (offset < length) {
-        langFontLink->GetStrCodePages(characters, length, acpCodePages, &actualCodePages, &numCharactersProcessed);
+        if (FAILED(langFontLink->GetStrCodePages(characters, length, acpCodePages, &actualCodePages, &numCharactersProcessed)))
+            return false;
         if ((actualCodePages & fontCodePages) == 0)
             return false;
         offset += numCharactersProcessed;
index c5e8b0d..8927058 100644 (file)
@@ -196,10 +196,15 @@ void UniscribeController::itemizeShapeAndPlace(const UChar* cp, unsigned length,
     // hanging out at the end of the array
     m_items.resize(6);
     int numItems = 0;
-    while (ScriptItemize(cp, length, m_items.size() - 1, &m_control, &m_state, m_items.data(), &numItems) == E_OUTOFMEMORY) {
+    HRESULT rc = S_OK;
+    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;
+    }
     m_items.resize(numItems + 1);
 
     if (m_run.rtl()) {
@@ -378,8 +383,12 @@ bool UniscribeController::shapeAndPlaceItem(const UChar* cp, unsigned i, const S
     while (m_computingOffsetPosition && m_offsetX >= leftEdge && m_offsetX < m_runWidthSoFar) {
         // The position is somewhere inside this run.
         int trailing = 0;
-        ScriptXtoCP(m_offsetX - leftEdge, clusters.size(), glyphs.size(), clusters.data(), visualAttributes.data(),
+        HRESULT rc = ::ScriptXtoCP(m_offsetX - leftEdge, clusters.size(), glyphs.size(), clusters.data(), visualAttributes.data(),
                     advances.data(), &item.a, &m_offsetPosition, &trailing);
+        if (FAILED(rc)) {
+            WTFLogAlways("UniscribeController::shapeAndPlaceItem: ScriptXtoCP failed rc=%lx", rc);
+            return true;
+        }
         if (trailing && m_includePartialGlyphs && m_offsetPosition < len - 1) {
             m_offsetPosition += m_currentCharacter + m_items[i].iCharPos;
             m_offsetX += m_run.rtl() ? -trailing : trailing;
index bcbbd4e..1fbe02c 100644 (file)
 typedef const struct __CFString * CFStringRef;
 #endif
 
+#ifndef _PREFAST_
 typedef wchar_t* BSTR;
+#else // _PREFAST_
+typedef _Null_terminated_ wchar_t* BSTR;
+#endif
 
 namespace WebCore {
 
index c6b2e97..f264b08 100644 (file)
 #include <wtf/Assertions.h>
 #include <wtf/HashTraits.h>
 
-typedef long HRESULT;
+#ifdef __midl
+typedef LONG HRESULT;
+#else
+typedef _Return_type_success_(return >= 0) long HRESULT;
+#endif // __midl
 
 // FIXME: Should we put this into the WebCore namespace and use "using" on it
 // as we do with things in WTF? 
index 0cca690..ad25989 100644 (file)
@@ -49,10 +49,10 @@ GDIObject<HBITMAP> allocImage(HDC dc, IntSize size, CGContextRef *targetRef)
 {
     BitmapInfo bmpInfo = BitmapInfo::create(size);
 
-    LPVOID bits;
+    LPVOID bits = nullptr;
     auto hbmp = adoptGDIObject(::CreateDIBSection(dc, &bmpInfo, DIB_RGB_COLORS, &bits, 0, 0));
 
-    if (!targetRef)
+    if (!targetRef || !hbmp)
         return hbmp;
 
     CGContextRef bitmapContext = CGBitmapContextCreate(bits, bmpInfo.bmiHeader.biWidth, bmpInfo.bmiHeader.biHeight, 8,
index 5546ba4..a956401 100644 (file)
@@ -160,7 +160,8 @@ void PopupMenuWin::show(const IntRect& r, FrameView* view, int index)
             setFocusedIndex(index);
     }
 
-    ::SystemParametersInfo(SPI_GETCOMBOBOXANIMATION, 0, &shouldAnimate, 0);
+    if (!::SystemParametersInfo(SPI_GETCOMBOBOXANIMATION, 0, &shouldAnimate, 0))
+        shouldAnimate = FALSE;
 
     if (shouldAnimate) {
         RECT viewRect = {0};
@@ -943,7 +944,8 @@ LRESULT PopupMenuWin::wndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lPa
             }
 
             BOOL shouldHotTrack = FALSE;
-            ::SystemParametersInfo(SPI_GETHOTTRACKING, 0, &shouldHotTrack, 0);
+            if (!::SystemParametersInfo(SPI_GETHOTTRACKING, 0, &shouldHotTrack, 0))
+                shouldHotTrack = FALSE;
 
             RECT bounds;
             GetClientRect(popupHandle(), &bounds);