Remove CFX_FormatString::Release()

xfa/fxfa/parser/xfa_localevalue.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 "xfa/fxfa/parser/xfa_localevalue.h"
 
 #include "core/fxcrt/fx_ext.h"
+#include "third_party/base/ptr_util.h"
 #include "xfa/fgas/localization/fgas_localeimp.h"
 #include "xfa/fxfa/parser/cxfa_document.h"
 #include "xfa/fxfa/parser/xfa_localemgr.h"
 #include "xfa/fxfa/parser/xfa_object.h"
 #include "xfa/fxfa/parser/xfa_utils.h"
 
 static const FX_DOUBLE fraction_scales[] = {0.1,
                                             0.01,
                                             0.001,
                                             0.0001,
@@ skipping 66 matching lines @@
       case XFA_VT_DATE:
         return FX_LOCALECATEGORY_Date;
       case XFA_VT_TIME:
         return FX_LOCALECATEGORY_Time;
       case XFA_VT_DATETIME:
         return FX_LOCALECATEGORY_DateTime;
     }
   }
   return eCategory;
 }
+
 bool CXFA_LocaleValue::ValidateValue(const CFX_WideString& wsValue,
                                      const CFX_WideString& wsPattern,
                                      IFX_Locale* pLocale,
                                      CFX_WideString* pMatchFormat) {
   CFX_WideString wsOutput;
   IFX_Locale* locale = m_pLocaleMgr->GetDefLocale();
-  if (pLocale) {
+  if (pLocale)
     m_pLocaleMgr->SetDefLocale(pLocale);
-  }
-  CFX_FormatString* pFormat = nullptr;
-  if (m_pLocaleMgr)

[Tom Sepez, 2016/12/07 22:56:13]

segv at 110 unless this block executes.

[dsinclair, 2016/12/07 23:17:43]

Do we know if m_pLocalMgr can be null? Should we be early returning or something
if it is? Does CFX_FormatString handle the nullptr?

[Tom Sepez, 2016/12/07 23:36:36]

Hope not.  Should we early return ?  Not unless we get crashes. The format
string method called here doesn't use this.

[dsinclair, 2016/12/07 23:45:20]

If CFX_FormatString doesn't need it, can we just remove the param from the
constructor?

[Tom Sepez, 2016/12/07 23:46:12]

Also segv at 187, even if we manage to successfully run SplitFormatString with a
null |this| (doesn't use other memembers).

-    pFormat = new CFX_FormatString(m_pLocaleMgr, false);
 
+  auto pFormat = pdfium::MakeUnique<CFX_FormatString>(m_pLocaleMgr, false);
   CFX_WideStringArray wsPatterns;
   pFormat->SplitFormatString(wsPattern, wsPatterns);
+
   bool bRet = false;
   int32_t iCount = wsPatterns.GetSize();
   int32_t i = 0;
   for (; i < iCount && !bRet; i++) {
     CFX_WideString wsFormat = wsPatterns[i];
     FX_LOCALECATEGORY eCategory = pFormat->GetCategory(wsFormat);
     eCategory = XFA_ValugeCategory(eCategory, m_dwType);
     switch (eCategory) {
       case FX_LOCALECATEGORY_Null:
         bRet = pFormat->ParseNull(wsValue, wsFormat);
@@ skipping 53 matching lines @@
           bRet = pFormat->FormatDateTime(wsValue, wsFormat, wsOutput,
                                          FX_DATETIMETYPE_DateTime);
         }
         break;
       }
       default:
         bRet = false;
         break;
     }
   }
-  if (bRet && pMatchFormat) {
+  if (bRet && pMatchFormat)
     *pMatchFormat = wsPatterns[i - 1];
-  }
-  pFormat->Release();
-  if (pLocale) {
+
+  if (pLocale)
     m_pLocaleMgr->SetDefLocale(locale);
-  }
+
   return bRet;
 }
+
 CFX_WideString CXFA_LocaleValue::GetValue() const {
   return m_wsValue;
 }
 uint32_t CXFA_LocaleValue::GetType() const {
   return m_dwType;
 }
 void CXFA_LocaleValue::SetValue(const CFX_WideString& wsValue,
                                 uint32_t dwType) {
   m_wsValue = wsValue;
   m_dwType = dwType;
@@ skipping 248 matching lines @@
     m_wsValue += wsTemp;
   }
   return true;
 }
 bool CXFA_LocaleValue::SetDateTime(const CFX_WideString& wsDateTime,
                                    const CFX_WideString& wsFormat,
                                    IFX_Locale* pLocale) {
   m_dwType = XFA_VT_DATETIME;
   return m_bValid = ParsePatternValue(wsDateTime, wsFormat, pLocale);
 }
+
 bool CXFA_LocaleValue::FormatPatterns(CFX_WideString& wsResult,
                                       const CFX_WideString& wsFormat,
                                       IFX_Locale* pLocale,
                                       XFA_VALUEPICTURE eValueType) const {
-  wsResult.clear();
-  bool bRet = false;
-
-  CFX_FormatString* pFormat = nullptr;
-  if (m_pLocaleMgr)

[Tom Sepez, 2016/12/07 22:56:13]

segv at 473 unless this block executes.

[dsinclair, 2016/12/07 23:17:43]

ditto

-    pFormat = new CFX_FormatString(m_pLocaleMgr, false);
-
+  auto pFormat = pdfium::MakeUnique<CFX_FormatString>(m_pLocaleMgr, false);
   CFX_WideStringArray wsPatterns;
   pFormat->SplitFormatString(wsFormat, wsPatterns);
+  wsResult.clear();
   int32_t iCount = wsPatterns.GetSize();
   for (int32_t i = 0; i < iCount; i++) {
-    bRet = FormatSinglePattern(wsResult, wsPatterns[i], pLocale, eValueType);
-    if (bRet) {
-      break;
-    }
+    if (FormatSinglePattern(wsResult, wsPatterns[i], pLocale, eValueType))
+      return true;
   }
-  pFormat->Release();
-  return bRet;
+  return false;
 }
+
 bool CXFA_LocaleValue::FormatSinglePattern(CFX_WideString& wsResult,
                                            const CFX_WideString& wsFormat,
                                            IFX_Locale* pLocale,
                                            XFA_VALUEPICTURE eValueType) const {
   IFX_Locale* locale = m_pLocaleMgr->GetDefLocale();
-  if (pLocale) {
+  if (pLocale)
     m_pLocaleMgr->SetDefLocale(pLocale);
-  }
+
   wsResult.clear();
   bool bRet = false;
-
-  CFX_FormatString* pFormat = nullptr;
-  if (m_pLocaleMgr)

[Tom Sepez, 2016/12/07 22:56:13]

segv at 534 unless this block executes

[dsinclair, 2016/12/07 23:17:43]

"

-    pFormat = new CFX_FormatString(m_pLocaleMgr, false);
-
+  auto pFormat = pdfium::MakeUnique<CFX_FormatString>(m_pLocaleMgr, false);
   FX_LOCALECATEGORY eCategory = pFormat->GetCategory(wsFormat);
   eCategory = XFA_ValugeCategory(eCategory, m_dwType);
   switch (eCategory) {
     case FX_LOCALECATEGORY_Null:
       if (m_wsValue.IsEmpty()) {
         bRet = pFormat->FormatNull(wsFormat, wsResult);
       }
       break;
     case FX_LOCALECATEGORY_Zero:
       if (m_wsValue == FX_WSTRC(L"0")) {
@@ skipping 15 matching lines @@
                                      FX_DATETIMETYPE_Time);
       break;
     case FX_LOCALECATEGORY_DateTime:
       bRet = pFormat->FormatDateTime(m_wsValue, wsFormat, wsResult,
                                      FX_DATETIMETYPE_DateTime);
       break;
     default:
       wsResult = m_wsValue;
       bRet = true;
   }
-  pFormat->Release();
   if (!bRet && (eCategory != FX_LOCALECATEGORY_Num ||
                 eValueType != XFA_VALUEPICTURE_Display)) {
     wsResult = m_wsValue;
   }
-  if (pLocale) {
+  if (pLocale)
     m_pLocaleMgr->SetDefLocale(locale);
-  }
+
   return bRet;
 }
+
 static bool XFA_ValueSplitDateTime(const CFX_WideString& wsDateTime,
                                    CFX_WideString& wsDate,
                                    CFX_WideString& wsTime) {
   wsDate = L"";
   wsTime = L"";
   if (wsDateTime.IsEmpty()) {
     return false;
   }
   int nSplitIndex = -1;
   nSplitIndex = wsDateTime.Find('T');
@@ skipping 238 matching lines @@
   }
   wsDate = wsDateTime.Left(nSplitIndex);
   wsTime = wsDateTime.Right(wsDateTime.GetLength() - nSplitIndex - 1);
   CFX_Unitime dt;
   return ValidateCanonicalDate(wsDate, dt) && ValidateCanonicalTime(wsTime);
 }
 bool CXFA_LocaleValue::ParsePatternValue(const CFX_WideString& wsValue,
                                          const CFX_WideString& wsPattern,
                                          IFX_Locale* pLocale) {
   IFX_Locale* locale = m_pLocaleMgr->GetDefLocale();
-  if (pLocale) {
+  if (pLocale)
     m_pLocaleMgr->SetDefLocale(pLocale);
-  }
 
-  CFX_FormatString* pFormat = nullptr;
-  if (m_pLocaleMgr)
-    pFormat = new CFX_FormatString(m_pLocaleMgr, false);

[Tom Sepez, 2016/12/07 22:56:13]

segv at 811, 881 unless this block executes

[dsinclair, 2016/12/07 23:17:43]

"

-
+  auto pFormat = pdfium::MakeUnique<CFX_FormatString>(m_pLocaleMgr, false);
   CFX_WideStringArray wsPatterns;
   pFormat->SplitFormatString(wsPattern, wsPatterns);
   bool bRet = false;
   int32_t iCount = wsPatterns.GetSize();
   for (int32_t i = 0; i < iCount && !bRet; i++) {
     CFX_WideString wsFormat = wsPatterns[i];
     FX_LOCALECATEGORY eCategory = pFormat->GetCategory(wsFormat);
     eCategory = XFA_ValugeCategory(eCategory, m_dwType);
     switch (eCategory) {
       case FX_LOCALECATEGORY_Null:
@@ skipping 48 matching lines @@
           SetDateTime(dt);
         }
         break;
       }
       default:
         m_wsValue = wsValue;
         bRet = true;
         break;
     }
   }
-  if (!bRet) {
+  if (!bRet)
     m_wsValue = wsValue;
-  }
-  pFormat->Release();
-  if (pLocale) {
+
+  if (pLocale)
     m_pLocaleMgr->SetDefLocale(locale);
-  }
+
   return bRet;
 }
+
 void CXFA_LocaleValue::GetNumbericFormat(CFX_WideString& wsFormat,
                                          int32_t nIntLen,
                                          int32_t nDecLen,
                                          bool bSign) {
   ASSERT(wsFormat.IsEmpty());
   ASSERT(nIntLen >= -1 && nDecLen >= -1);
   int32_t nTotalLen = (nIntLen >= 0 ? nIntLen : 2) + (bSign ? 1 : 0) +
                       (nDecLen >= 0 ? nDecLen : 2) + (nDecLen == 0 ? 0 : 1);
   FX_WCHAR* lpBuf = wsFormat.GetBuffer(nTotalLen);
   int32_t nPos = 0;
@@ skipping 90 matching lines @@
       } else if (cf == L'z') {
         nf++;
       } else {
         return false;
       }
     }
     n++;
   }
   return n == nCount;
 }