commit a8c33d4f014f592322608fe07f1909ae2f7e886f
author Lei Zhang <thestig@chromium.org> Thu Oct 22 20:56:58 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Thu Oct 22 20:56:58 2020 +0000
tree fbfa6b19c1a3b030cc07ab274d6fffccb5e8fdab
parent 616a426b3cbfcb15bd90c0f8ba0436edf71e08f7

Replace manual memory allocation / deletion with a unique_ptr.

Add ScopedDIBitmap, which is std::unique_ptr with a custom deleter, to
remove a case with manual memory management.

Also fix some nits along the way:
- Rename (anonymous_namespace)::LoadDIBitmap(), so it does not clash
  with CGdiplusExt::LoadDIBitmap().
- Remove / more narrowly scope a couple of variables.

Change-Id: I1b5d331c1222c0a8bd23e3ec61eef575a464864c
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75570
Reviewed-by: Hui Yingst <nigi@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fxge/win32/cgdi_plus_ext.cpp

diff --git a/core/fxge/win32/cgdi_plus_ext.cpp b/core/fxge/win32/cgdi_plus_ext.cpp
index c5d48d2..8dcb453 100644
--- a/core/fxge/win32/cgdi_plus_ext.cpp
+++ b/core/fxge/win32/cgdi_plus_ext.cpp
@@ -659,28 +659,45 @@
   GpStream* pStream;
 };
 
-PREVIEW3_DIBITMAP* LoadDIBitmap(WINDIB_Open_Args_ args) {
+struct DIBitmapDeleter {
+  void operator()(PREVIEW3_DIBITMAP* bitmap) const {
+    const CGdiplusExt& GdiplusExt = GetGdiplusExt();
+    CallFunc(GdipBitmapUnlockBits)(bitmap->pBitmap, bitmap->pBitmapData);
+    CallFunc(GdipDisposeImage)(bitmap->pBitmap);
+    FX_Free(bitmap->pBitmapData);
+    FX_Free((LPBYTE)bitmap->pbmi);
+    if (bitmap->pStream)
+      bitmap->pStream->Release();
+    FX_Free(bitmap);
+  }
+};
+
+using ScopedDIBitmap = std::unique_ptr<PREVIEW3_DIBITMAP, DIBitmapDeleter>;
+
+ScopedDIBitmap LoadDIBitmapImpl(WINDIB_Open_Args_ args) {
   Gdiplus::GpBitmap* pBitmap;
   GpStream* pStream = nullptr;
   const CGdiplusExt& GdiplusExt = GetGdiplusExt();
-  Gdiplus::Status status = Gdiplus::Ok;
   if (args.flags == WINDIB_OPEN_PATHNAME) {
-    status = CallFunc(GdipCreateBitmapFromFileICM)(args.path_name, &pBitmap);
+    if (CallFunc(GdipCreateBitmapFromFileICM)(args.path_name, &pBitmap) !=
+        Gdiplus::Ok) {
+      return nullptr;
+    }
   } else {
     if (args.memory_size == 0 || !args.memory_base)
       return nullptr;
 
     pStream = new GpStream;
     pStream->Write(args.memory_base, (ULONG)args.memory_size, nullptr);
-    status = CallFunc(GdipCreateBitmapFromStreamICM)(pStream, &pBitmap);
-  }
-  if (status != Gdiplus::Ok) {
-    if (pStream)
+    if (CallFunc(GdipCreateBitmapFromStreamICM)(pStream, &pBitmap) !=
+        Gdiplus::Ok) {
       pStream->Release();
-
-    return nullptr;
+      return nullptr;
+    }
   }
-  UINT height, width;
+
+  UINT height;
+  UINT width;
   CallFunc(GdipGetImageHeight)(pBitmap, &height);
   CallFunc(GdipGetImageWidth)(pBitmap, &width);
   Gdiplus::PixelFormat pixel_format;
@@ -727,7 +744,7 @@
       ppal[i] = pal.Entries[i] & 0x00ffffff;
     }
   }
-  PREVIEW3_DIBITMAP* pInfo = FX_Alloc(PREVIEW3_DIBITMAP, 1);
+  ScopedDIBitmap pInfo(FX_Alloc(PREVIEW3_DIBITMAP, 1));
   pInfo->pbmi = (BITMAPINFO*)buf;
   pInfo->pScan0 = (LPBYTE)pBitmapData->Scan0;
   pInfo->Stride = pBitmapData->Stride;
@@ -737,17 +754,6 @@
   return pInfo;
 }
 
-void FreeDIBitmap(PREVIEW3_DIBITMAP* pInfo) {
-  const CGdiplusExt& GdiplusExt = GetGdiplusExt();
-  CallFunc(GdipBitmapUnlockBits)(pInfo->pBitmap, pInfo->pBitmapData);
-  CallFunc(GdipDisposeImage)(pInfo->pBitmap);
-  FX_Free(pInfo->pBitmapData);
-  FX_Free((LPBYTE)pInfo->pbmi);
-  if (pInfo->pStream)
-    pInfo->pStream->Release();
-  FX_Free(pInfo);
-}
-
 }  // namespace
 
 CGdiplusExt::CGdiplusExt() = default;
@@ -975,7 +981,7 @@
 }
 
 RetainPtr<CFX_DIBitmap> CGdiplusExt::LoadDIBitmap(WINDIB_Open_Args_ args) {
-  PREVIEW3_DIBITMAP* pInfo = ::LoadDIBitmap(args);
+  ScopedDIBitmap pInfo = LoadDIBitmapImpl(args);
   if (!pInfo)
     return nullptr;
 
@@ -994,6 +1000,5 @@
   RetainPtr<CFX_DIBitmap> pDIBitmap =
       CFX_WindowsDIB::LoadFromBuf(pInfo->pbmi, pData);
   FX_Free(pData);
-  FreeDIBitmap(pInfo);
   return pDIBitmap;
 }