Fix / simplify CFX_FolderFontInfo::GetFontData() behavior.

core/src/fxge/ge/fx_ge_fontmap.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include <limits>
 
 #include "../../../include/fxge/fx_ge.h"
 #include "../../../include/fxge/fx_freetype.h"
 #include "../fontdata/chromefontdata/chromefontdata.h"
 #include "text_int.h"
+#include "third_party/base/stl_util.h"
 
 #define GET_TT_SHORT(w) (FX_WORD)(((w)[0] << 8) | (w)[1])
 #define GET_TT_LONG(w) \
   (FX_DWORD)(((w)[0] << 24) | ((w)[1] << 16) | ((w)[2] << 8) | (w)[3])
 
 namespace {
 
 struct BuiltinFont {
   const uint8_t* m_pFontData;
   FX_DWORD m_dwSize;
@@ skipping 14 matching lines @@
     {g_FoxitSerifItalicFontData, 21227},
     {g_FoxitSymbolFontData, 16729},
     {g_FoxitDingbatsFontData, 29513},
 };
 
 const BuiltinFont g_MMFonts[2] = {
     {g_FoxitSerifMMFontData, 113417},
     {g_FoxitSansMMFontData, 66919},
 };
 
+const FX_DWORD kTableNAME = 0x6e616d65;

[Tom Sepez, 2015/11/09 18:27:51]

nit: again, want to use a macro to make a big-endian value from "name".  I think
its FXDWORD_GET_MSBFIRST("name");

[Lei Zhang, 2015/11/09 19:20:00]

Done.

I locally added a static_assert(kFoo, kFooMacro, "fail") to sanity check before
making the change. However, FXDWORD_GET_MSBFIRST() as is did not play well with
static_assert, so I simplified FXDWORD_GET_MSBFIRST().

+const FX_DWORD kTableTTCF = 0x74746366;

[Tom Sepez, 2015/11/09 18:27:51]

nit: and FXDWORD_GET_MSBFIRST("ttcf")
but doublecheck my arithmetic/translations.

[Lei Zhang, 2015/11/09 19:20:00]

Done.

+
 CFX_ByteString KeyNameFromFace(const CFX_ByteString& face_name,
                                int weight,
                                FX_BOOL bItalic) {
   CFX_ByteString key(face_name);
   key += ',';
   key += CFX_ByteString::FormatInteger(weight);
   key += bItalic ? 'I' : 'N';
   return key;
 }
 
@@ skipping 450 matching lines @@
       CFX_ByteString buffer;
       if (!pFile->ReadBlock(buffer.GetBuffer(size), offset, size)) {
         return CFX_ByteString();
       }
       buffer.ReleaseBuffer(size);
       return buffer;
     }
   }
   return CFX_ByteString();
 }
+
 CFX_ByteString CFX_FontMapper::GetPSNameFromTT(void* hFont) {
-  if (m_pFontInfo == NULL) {
-    return CFX_ByteString();
-  }
   CFX_ByteString result;

[Tom Sepez, 2015/11/09 18:27:51]

nit: no need for "result" local, just return CFX_ByteString() during the early
returns, or GetNameFromTT() directly.

[Lei Zhang, 2015/11/09 19:20:00]

Done. Some say, it makes it easier for compilers to do RVO, but I'll just assume
compilers will be smart enough to figure it out.

-  FX_DWORD size = m_pFontInfo->GetFontData(hFont, 0x6e616d65, NULL, 0);
-  if (size) {
-    uint8_t* buffer = FX_Alloc(uint8_t, size);
-    m_pFontInfo->GetFontData(hFont, 0x6e616d65, buffer, size);
-    result = GetNameFromTT(buffer, 6);
-    FX_Free(buffer);
-  }
+  if (!m_pFontInfo)
+    return result;
+
+  FX_DWORD size = m_pFontInfo->GetFontData(hFont, kTableNAME, nullptr, 0);
+  if (!size)
+    return result;
+
+  std::vector<uint8_t> buffer(size);
+  uint8_t* buffer_ptr = pdfium::vector_as_array(&buffer);
+  FX_DWORD bytes_read =
+      m_pFontInfo->GetFontData(hFont, kTableNAME, buffer_ptr, size);
+  if (bytes_read == size)
+    result = GetNameFromTT(buffer_ptr, 6);

[Tom Sepez, 2015/11/09 18:27:51]

nit: prefer early return on error.

[Lei Zhang, 2015/11/09 19:20:00]

Acknowledged.

   return result;
 }
+
 void CFX_FontMapper::AddInstalledFont(const CFX_ByteString& name, int charset) {
   if (m_pFontInfo == NULL) {
     return;
   }
   if (m_CharsetArray.Find((FX_DWORD)charset) == -1) {
     m_CharsetArray.Add((FX_DWORD)charset);
     m_FaceArray.Add(name);
   }
   if (name == m_LastFamily) {
     return;
@@ skipping 564 matching lines @@
       }
       int index = m_CharsetArray.Find(Charset);
       if (index < 0) {
         return UseInternalSubst(pSubstFont, iBaseFont, italic_angle, old_weight,
                                 PitchFamily);
       }
       hFont = m_pFontInfo->GetFont(m_FaceArray[index]);
     }
   }
   pSubstFont->m_ExtHandle = m_pFontInfo->RetainFont(hFont);
-  if (hFont == NULL) {
-    return NULL;
-  }
+  if (!hFont)
+    return nullptr;
+
   m_pFontInfo->GetFaceName(hFont, SubstName);
   if (Charset == FXFONT_DEFAULT_CHARSET) {
     m_pFontInfo->GetFontCharset(hFont, Charset);
   }
-  FX_DWORD ttc_size = m_pFontInfo->GetFontData(hFont, 0x74746366, NULL, 0);
-  FX_DWORD font_size = m_pFontInfo->GetFontData(hFont, 0, NULL, 0);
+  FX_DWORD ttc_size = m_pFontInfo->GetFontData(hFont, kTableTTCF, nullptr, 0);
+  FX_DWORD font_size = m_pFontInfo->GetFontData(hFont, 0, nullptr, 0);
   if (font_size == 0 && ttc_size == 0) {
     m_pFontInfo->DeleteFont(hFont);
-    return NULL;
+    return nullptr;
   }
-  FXFT_Face face = NULL;
+  FXFT_Face face = nullptr;
   if (ttc_size) {
     uint8_t temp[1024];
-    m_pFontInfo->GetFontData(hFont, 0x74746366, temp, 1024);
+    m_pFontInfo->GetFontData(hFont, kTableTTCF, temp, 1024);
     FX_DWORD checksum = 0;
     for (int i = 0; i < 256; i++) {
       checksum += ((FX_DWORD*)temp)[i];
     }
     uint8_t* pFontData;
     face = m_pFontMgr->GetCachedTTCFace(ttc_size, checksum,
                                         ttc_size - font_size, pFontData);
     if (face == NULL) {
       pFontData = FX_Alloc(uint8_t, ttc_size);
-      m_pFontInfo->GetFontData(hFont, 0x74746366, pFontData, ttc_size);
+      m_pFontInfo->GetFontData(hFont, kTableTTCF, pFontData, ttc_size);
       face = m_pFontMgr->AddCachedTTCFace(ttc_size, checksum, pFontData,
                                           ttc_size, ttc_size - font_size);
     }
   } else {
     uint8_t* pFontData;
     face = m_pFontMgr->GetCachedFace(SubstName, weight, bItalic, pFontData);
     if (face == NULL) {
       pFontData = FX_Alloc(uint8_t, font_size);
       m_pFontInfo->GetFontData(hFont, 0, pFontData, font_size);
       face = m_pFontMgr->AddCachedFace(SubstName, weight, bItalic, pFontData,
@@ skipping 108 matching lines @@
   FXSYS_fseek(pFile, 0, FXSYS_SEEK_END);
   FX_DWORD filesize = FXSYS_ftell(pFile);
   uint8_t buffer[16];
   FXSYS_fseek(pFile, 0, FXSYS_SEEK_SET);
   size_t readCnt = FXSYS_fread(buffer, 12, 1, pFile);
   if (readCnt != 1) {
     FXSYS_fclose(pFile);
     return;
   }
 
-  if (GET_TT_LONG(buffer) == 0x74746366) {
+  if (GET_TT_LONG(buffer) == kTableTTCF) {
     FX_DWORD nFaces = GET_TT_LONG(buffer + 8);
     if (nFaces > std::numeric_limits<FX_DWORD>::max() / 4) {
       FXSYS_fclose(pFile);
       return;
     }
     FX_DWORD face_bytes = nFaces * 4;
     uint8_t* offsets = FX_Alloc(uint8_t, face_bytes);
     readCnt = FXSYS_fread(offsets, 1, face_bytes, pFile);
     if (readCnt != face_bytes) {
       FX_Free(offsets);
@@ skipping 18 matching lines @@
   char buffer[16];
   if (!FXSYS_fread(buffer, 12, 1, pFile)) {
     return;
   }
   FX_DWORD nTables = GET_TT_SHORT(buffer + 4);
   CFX_ByteString tables = _FPDF_ReadStringFromFile(pFile, nTables * 16);
   if (tables.IsEmpty()) {
     return;
   }
   CFX_ByteString names =
-      _FPDF_LoadTableFromTT(pFile, tables, nTables, 0x6e616d65);
+      _FPDF_LoadTableFromTT(pFile, tables, nTables, kTableNAME);
   CFX_ByteString facename = GetNameFromTT(names, 1);
   CFX_ByteString style = GetNameFromTT(names, 2);
   if (style != "Regular") {
     facename += " " + style;
   }
   if (m_FontList.find(facename) != m_FontList.end()) {
     return;
   }
   CFX_FontFaceInfo* pInfo =
       new CFX_FontFaceInfo(path, facename, tables, offset, filesize);
@@ skipping 142 matching lines @@
                                   int charset,
                                   int pitch_family,
                                   const FX_CHAR* family,
                                   int& iExact) {
   return NULL;
 }
 void* CFX_FolderFontInfo::GetFont(const FX_CHAR* face) {
   auto it = m_FontList.find(face);
   return it != m_FontList.end() ? it->second : nullptr;
 }
+
 FX_DWORD CFX_FolderFontInfo::GetFontData(void* hFont,
                                          FX_DWORD table,
                                          uint8_t* buffer,
                                          FX_DWORD size) {
-  if (hFont == NULL) {
+  if (!hFont)
     return 0;
-  }
-  CFX_FontFaceInfo* pFont = (CFX_FontFaceInfo*)hFont;
-  FXSYS_FILE* pFile = NULL;
-  if (size > 0) {
-    pFile = FXSYS_fopen(pFont->m_FilePath, "rb");
-    if (pFile == NULL) {
-      return 0;
-    }
-  }
+
+  const CFX_FontFaceInfo* pFont = static_cast<CFX_FontFaceInfo*>(hFont);
   FX_DWORD datasize = 0;
   FX_DWORD offset = 0;
   if (table == 0) {
     datasize = pFont->m_FontOffset ? 0 : pFont->m_FileSize;
-  } else if (table == 0x74746366) {
+  } else if (table == kTableTTCF) {
     datasize = pFont->m_FontOffset ? pFont->m_FileSize : 0;
   } else {
     FX_DWORD nTables = pFont->m_FontTables.GetLength() / 16;
     for (FX_DWORD i = 0; i < nTables; i++) {
-      const uint8_t* p = (const uint8_t*)pFont->m_FontTables + i * 16;
+      const uint8_t* p =
+          static_cast<const uint8_t*>(pFont->m_FontTables) + i * 16;
       if (GET_TT_LONG(p) == table) {
         offset = GET_TT_LONG(p + 8);
         datasize = GET_TT_LONG(p + 12);
       }
     }
   }
-  if (datasize && size >= datasize && pFile) {
-    if (FXSYS_fseek(pFile, offset, FXSYS_SEEK_SET) < 0 ||
-        FXSYS_fread(buffer, datasize, 1, pFile) != 1) {
-      datasize = 0;
-    }
+
+  if (!datasize || size < datasize)
+    return datasize;
+
+  FXSYS_FILE* pFile = FXSYS_fopen(pFont->m_FilePath, "rb");
+  if (!pFile)
+    return 0;
+
+  if (FXSYS_fseek(pFile, offset, FXSYS_SEEK_SET) < 0 ||
+      FXSYS_fread(buffer, datasize, 1, pFile) != 1) {
+    datasize = 0;
   }
-  if (pFile) {
-    FXSYS_fclose(pFile);
-  }
+  FXSYS_fclose(pFile);
   return datasize;
 }
+
 void CFX_FolderFontInfo::DeleteFont(void* hFont) {}
 FX_BOOL CFX_FolderFontInfo::GetFaceName(void* hFont, CFX_ByteString& name) {
   if (hFont == NULL) {
     return FALSE;
   }
   CFX_FontFaceInfo* pFont = (CFX_FontFaceInfo*)hFont;
   name = pFont->m_FaceName;
   return TRUE;
 }
 FX_BOOL CFX_FolderFontInfo::GetFontCharset(void* hFont, int& charset) {
   return FALSE;
 }
 
 int PDF_GetStandardFontName(CFX_ByteString* name) {
   AltFontName* found = static_cast<AltFontName*>(
       FXSYS_bsearch(name->c_str(), g_AltFontNames, FX_ArraySize(g_AltFontNames),
                     sizeof(AltFontName), compareString));
   if (!found)
     return -1;
 
   *name = g_Base14FontNames[found->m_Index];
   return found->m_Index;
 }