Remove gotos in JBig2 code.

core/src/fxcodec/jbig2/JBig2_Context.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 "JBig2_Context.h"
 
 #include <list>
 
@@ skipping 407 matching lines @@
       m_pStream->offset(pSegment->m_dwData_length);
       break;
     default:
       break;
   }
   return JBIG2_SUCCESS;
 }
 
 int32_t CJBig2_Context::parseSymbolDict(CJBig2_Segment* pSegment,
                                         IFX_Pause* pPause) {
-  FX_DWORD dwTemp;
   FX_WORD wFlags;
-  uint8_t cSDHUFFDH, cSDHUFFDW, cSDHUFFBMSIZE, cSDHUFFAGGINST;
-  CJBig2_HuffmanTable* Table_B1 = nullptr;
-  CJBig2_HuffmanTable* Table_B2 = nullptr;
-  CJBig2_HuffmanTable* Table_B3 = nullptr;
-  CJBig2_HuffmanTable* Table_B4 = nullptr;
-  CJBig2_HuffmanTable* Table_B5 = nullptr;
-  int32_t i, nIndex, nRet;
-  CJBig2_Segment* pSeg = nullptr;
-  CJBig2_Segment* pLRSeg = nullptr;
-  FX_BOOL bUsed;
-  CJBig2_Image** SDINSYMS = nullptr;
-  JBig2ArithCtx* gbContext = nullptr;
-  JBig2ArithCtx* grContext = nullptr;
-  CJBig2_ArithDecoder* pArithDecoder;
-  CJBig2_SDDProc* pSymbolDictDecoder = new CJBig2_SDDProc();
-  const uint8_t* key = pSegment->m_pData;
-  FX_BOOL cache_hit = false;
-  if (m_pStream->readShortInteger(&wFlags) != 0) {
-    nRet = JBIG2_ERROR_TOO_SHORT;
-    goto failed;
-  }
+  if (m_pStream->readShortInteger(&wFlags) != 0)
+    return JBIG2_ERROR_TOO_SHORT;
+
+  nonstd::unique_ptr<CJBig2_SDDProc> pSymbolDictDecoder(new CJBig2_SDDProc);
   pSymbolDictDecoder->SDHUFF = wFlags & 0x0001;
   pSymbolDictDecoder->SDREFAGG = (wFlags >> 1) & 0x0001;
   pSymbolDictDecoder->SDTEMPLATE = (wFlags >> 10) & 0x0003;
   pSymbolDictDecoder->SDRTEMPLATE = (wFlags >> 12) & 0x0003;
-  cSDHUFFDH = (wFlags >> 2) & 0x0003;
-  cSDHUFFDW = (wFlags >> 4) & 0x0003;
-  cSDHUFFBMSIZE = (wFlags >> 6) & 0x0001;
-  cSDHUFFAGGINST = (wFlags >> 7) & 0x0001;
+  uint8_t cSDHUFFDH = (wFlags >> 2) & 0x0003;
+  uint8_t cSDHUFFDW = (wFlags >> 4) & 0x0003;
+  uint8_t cSDHUFFBMSIZE = (wFlags >> 6) & 0x0001;
+  uint8_t cSDHUFFAGGINST = (wFlags >> 7) & 0x0001;
+  FX_DWORD dwTemp;

[Tom Sepez, 2015/10/05 16:34:31]

Can we scope this to the next block?  That way, it is obvious that it can't be
used uninitialized later.  There are going to be a few other dwTemp declarations
required in other blocks as a result.

[Lei Zhang, 2015/10/07 07:51:11]

Done.

   if (pSymbolDictDecoder->SDHUFF == 0) {
     if (pSymbolDictDecoder->SDTEMPLATE == 0) {
       dwTemp = 8;

[Tom Sepez, 2015/10/05 16:34:32]

nit: this becomes a nice ? expr when combined with the decl.

[Lei Zhang, 2015/10/07 07:51:11]

Done.

     } else {
       dwTemp = 2;
     }
-    for (i = 0; i < (int32_t)dwTemp; i++) {
-      if (m_pStream->read1Byte((uint8_t*)&pSymbolDictDecoder->SDAT[i]) != 0) {
-        nRet = JBIG2_ERROR_TOO_SHORT;
-        goto failed;
-      }
+    for (int32_t i = 0; i < (int32_t)dwTemp; i++) {

[Tom Sepez, 2015/10/05 16:34:32]

nit: why not FX_DWORD i and lose the cast? also ++i.

[Lei Zhang, 2015/10/07 07:51:12]

Done.

+      if (m_pStream->read1Byte((uint8_t*)&pSymbolDictDecoder->SDAT[i]) != 0)
+        return JBIG2_ERROR_TOO_SHORT;
     }
   }
   if ((pSymbolDictDecoder->SDREFAGG == 1) &&
       (pSymbolDictDecoder->SDRTEMPLATE == 0)) {
-    for (i = 0; i < 4; i++) {
-      if (m_pStream->read1Byte((uint8_t*)&pSymbolDictDecoder->SDRAT[i]) != 0) {
-        nRet = JBIG2_ERROR_TOO_SHORT;
-        goto failed;
-      }
+    for (int32_t i = 0; i < 4; i++) {
+      if (m_pStream->read1Byte((uint8_t*)&pSymbolDictDecoder->SDRAT[i]) != 0)
+        return JBIG2_ERROR_TOO_SHORT;
     }
   }
   if ((m_pStream->readInteger(&pSymbolDictDecoder->SDNUMEXSYMS) != 0) ||
       (m_pStream->readInteger(&pSymbolDictDecoder->SDNUMNEWSYMS) != 0)) {
-    nRet = JBIG2_ERROR_TOO_SHORT;
-    goto failed;
+    return JBIG2_ERROR_TOO_SHORT;
   }
   if (pSymbolDictDecoder->SDNUMEXSYMS > JBIG2_MAX_EXPORT_SYSMBOLS ||
       pSymbolDictDecoder->SDNUMNEWSYMS > JBIG2_MAX_NEW_SYSMBOLS) {
-    nRet = JBIG2_ERROR_LIMIT;
-    goto failed;
+    return JBIG2_ERROR_LIMIT;
   }
-  for (i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
-    if (!findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i])) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
+  for (int32_t i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
+    if (!findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i]))
+      return JBIG2_ERROR_FATAL;
   }
+  CJBig2_Segment* pSeg = nullptr;

[Tom Sepez, 2015/10/05 16:34:32]

nit: pSeg maybe scopes tighter?  I think it's always re-assigned when used
later, but doublecheck.

[Lei Zhang, 2015/10/07 07:51:11]

Done.

+  CJBig2_Segment* pLRSeg = nullptr;
   pSymbolDictDecoder->SDNUMINSYMS = 0;
-  for (i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
+  for (int32_t i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
     pSeg = findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i]);
     if (pSeg->m_cFlags.s.type == 0) {
       pSymbolDictDecoder->SDNUMINSYMS += pSeg->m_Result.sd->SDNUMEXSYMS;
       pLRSeg = pSeg;
     }
   }
-  if (pSymbolDictDecoder->SDNUMINSYMS == 0) {
-    SDINSYMS = NULL;
-  } else {
-    SDINSYMS = FX_Alloc(CJBig2_Image*, pSymbolDictDecoder->SDNUMINSYMS);
+
+  nonstd::unique_ptr<CJBig2_Image*, FxFreeDeleter> SDINSYMS;
+  if (pSymbolDictDecoder->SDNUMINSYMS != 0) {
+    SDINSYMS.reset(FX_Alloc(CJBig2_Image*, pSymbolDictDecoder->SDNUMINSYMS));
     dwTemp = 0;
-    for (i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
+    for (int32_t i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
       pSeg = findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i]);
       if (pSeg->m_cFlags.s.type == 0) {
-        JBIG2_memcpy(SDINSYMS + dwTemp, pSeg->m_Result.sd->SDEXSYMS,
+        JBIG2_memcpy(SDINSYMS.get() + dwTemp, pSeg->m_Result.sd->SDEXSYMS,
                      pSeg->m_Result.sd->SDNUMEXSYMS * sizeof(CJBig2_Image*));
         dwTemp += pSeg->m_Result.sd->SDNUMEXSYMS;
       }
     }
   }
-  pSymbolDictDecoder->SDINSYMS = SDINSYMS;
+  pSymbolDictDecoder->SDINSYMS = SDINSYMS.get();
+
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B1;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B2;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B3;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B4;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B5;
   if (pSymbolDictDecoder->SDHUFF == 1) {
-    if ((cSDHUFFDH == 2) || (cSDHUFFDW == 2)) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
-    nIndex = 0;
+    if ((cSDHUFFDH == 2) || (cSDHUFFDW == 2))
+      return JBIG2_ERROR_FATAL;
+
+    int32_t nIndex = 0;
     if (cSDHUFFDH == 0) {
-      Table_B4 = new CJBig2_HuffmanTable(HuffmanTable_B4,
-                                         FX_ArraySize(HuffmanTable_B4),
-                                         HuffmanTable_HTOOB_B4);
-      pSymbolDictDecoder->SDHUFFDH = Table_B4;
+      Table_B4.reset(new CJBig2_HuffmanTable(HuffmanTable_B4,
+                                             FX_ArraySize(HuffmanTable_B4),
+                                             HuffmanTable_HTOOB_B4));
+      pSymbolDictDecoder->SDHUFFDH = Table_B4.get();
     } else if (cSDHUFFDH == 1) {
-      Table_B5 = new CJBig2_HuffmanTable(HuffmanTable_B5,
-                                         FX_ArraySize(HuffmanTable_B5),
-                                         HuffmanTable_HTOOB_B5);
-      pSymbolDictDecoder->SDHUFFDH = Table_B5;
+      Table_B5.reset(new CJBig2_HuffmanTable(HuffmanTable_B5,
+                                             FX_ArraySize(HuffmanTable_B5),
+                                             HuffmanTable_HTOOB_B5));
+      pSymbolDictDecoder->SDHUFFDH = Table_B5.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pSymbolDictDecoder->SDHUFFDH = pSeg->m_Result.ht;
     }
     if (cSDHUFFDW == 0) {
-      Table_B2 = new CJBig2_HuffmanTable(HuffmanTable_B2,
-                                         FX_ArraySize(HuffmanTable_B2),
-                                         HuffmanTable_HTOOB_B2);
-      pSymbolDictDecoder->SDHUFFDW = Table_B2;
+      Table_B2.reset(new CJBig2_HuffmanTable(HuffmanTable_B2,
+                                             FX_ArraySize(HuffmanTable_B2),
+                                             HuffmanTable_HTOOB_B2));
+      pSymbolDictDecoder->SDHUFFDW = Table_B2.get();
     } else if (cSDHUFFDW == 1) {
-      Table_B3 = new CJBig2_HuffmanTable(HuffmanTable_B3,
-                                         FX_ArraySize(HuffmanTable_B3),
-                                         HuffmanTable_HTOOB_B3);
-      pSymbolDictDecoder->SDHUFFDW = Table_B3;
+      Table_B3.reset(new CJBig2_HuffmanTable(HuffmanTable_B3,
+                                             FX_ArraySize(HuffmanTable_B3),
+                                             HuffmanTable_HTOOB_B3));
+      pSymbolDictDecoder->SDHUFFDW = Table_B3.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pSymbolDictDecoder->SDHUFFDW = pSeg->m_Result.ht;
     }
     if (cSDHUFFBMSIZE == 0) {
-      Table_B1 = new CJBig2_HuffmanTable(HuffmanTable_B1,
-                                         FX_ArraySize(HuffmanTable_B1),
-                                         HuffmanTable_HTOOB_B1);
-      pSymbolDictDecoder->SDHUFFBMSIZE = Table_B1;
+      Table_B1.reset(new CJBig2_HuffmanTable(HuffmanTable_B1,
+                                             FX_ArraySize(HuffmanTable_B1),
+                                             HuffmanTable_HTOOB_B1));
+      pSymbolDictDecoder->SDHUFFBMSIZE = Table_B1.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pSymbolDictDecoder->SDHUFFBMSIZE = pSeg->m_Result.ht;
     }
     if (pSymbolDictDecoder->SDREFAGG == 1) {
       if (cSDHUFFAGGINST == 0) {
         if (!Table_B1) {
-          Table_B1 = new CJBig2_HuffmanTable(HuffmanTable_B1,
-                                             FX_ArraySize(HuffmanTable_B1),
-                                             HuffmanTable_HTOOB_B1);
+          Table_B1.reset(new CJBig2_HuffmanTable(HuffmanTable_B1,
+                                                 FX_ArraySize(HuffmanTable_B1),
+                                                 HuffmanTable_HTOOB_B1));
         }
-        pSymbolDictDecoder->SDHUFFAGGINST = Table_B1;
+        pSymbolDictDecoder->SDHUFFAGGINST = Table_B1.get();
       } else {
         pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-        if (!pSeg) {
-          nRet = JBIG2_ERROR_FATAL;
-          goto failed;
-        }
+        if (!pSeg)
+          return JBIG2_ERROR_FATAL;
         pSymbolDictDecoder->SDHUFFAGGINST = pSeg->m_Result.ht;
       }
     }
   }
+
+  nonstd::unique_ptr<JBig2ArithCtx, FxFreeDeleter> gbContext;
+  nonstd::unique_ptr<JBig2ArithCtx, FxFreeDeleter> grContext;
   if ((wFlags & 0x0100) && pLRSeg && pLRSeg->m_Result.sd->m_bContextRetained) {
     if (pSymbolDictDecoder->SDHUFF == 0) {
       dwTemp = pSymbolDictDecoder->SDTEMPLATE == 0
                    ? 65536
                    : pSymbolDictDecoder->SDTEMPLATE == 1 ? 8192 : 1024;
-      gbContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-      JBIG2_memcpy(gbContext, pLRSeg->m_Result.sd->m_gbContext,
+      gbContext.reset(FX_Alloc(JBig2ArithCtx, dwTemp));
+      JBIG2_memcpy(gbContext.get(), pLRSeg->m_Result.sd->m_gbContext,
                    sizeof(JBig2ArithCtx) * dwTemp);
     }
     if (pSymbolDictDecoder->SDREFAGG == 1) {
       dwTemp = pSymbolDictDecoder->SDRTEMPLATE ? 1 << 10 : 1 << 13;

[Tom Sepez, 2015/10/05 16:34:31]

nit: 8192 and 1024 as in line 574, but let's write it different and in a
different order without an  == 1.  Is this a bool? Are the tests for == 1 above
wrong?

[Lei Zhang, 2015/10/07 07:51:11]

I refactored some of this code, but I'll fix the bools in another pass.
Otherwise this CL may never end.

-      grContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-      JBIG2_memcpy(grContext, pLRSeg->m_Result.sd->m_grContext,
+      grContext.reset(FX_Alloc(JBig2ArithCtx, dwTemp));
+      JBIG2_memcpy(grContext.get(), pLRSeg->m_Result.sd->m_grContext,
                    sizeof(JBig2ArithCtx) * dwTemp);
     }
   } else {
     if (pSymbolDictDecoder->SDHUFF == 0) {
       dwTemp = pSymbolDictDecoder->SDTEMPLATE == 0
                    ? 65536
                    : pSymbolDictDecoder->SDTEMPLATE == 1 ? 8192 : 1024;
-      gbContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-      JBIG2_memset(gbContext, 0, sizeof(JBig2ArithCtx) * dwTemp);
+      gbContext.reset(FX_Alloc(JBig2ArithCtx, dwTemp));
+      JBIG2_memset(gbContext.get(), 0, sizeof(JBig2ArithCtx) * dwTemp);
     }
     if (pSymbolDictDecoder->SDREFAGG == 1) {
       dwTemp = pSymbolDictDecoder->SDRTEMPLATE ? 1 << 10 : 1 << 13;
-      grContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-      JBIG2_memset(grContext, 0, sizeof(JBig2ArithCtx) * dwTemp);
+      grContext.reset(FX_Alloc(JBig2ArithCtx, dwTemp));
+      JBIG2_memset(grContext.get(), 0, sizeof(JBig2ArithCtx) * dwTemp);
     }
   }
+  const uint8_t* key = pSegment->m_pData;
+  FX_BOOL cache_hit = false;
   pSegment->m_nResultType = JBIG2_SYMBOL_DICT_POINTER;
   for (std::list<CJBig2_CachePair>::iterator it = m_pSymbolDictCache->begin();

[Tom Sepez, 2015/10/05 16:34:32]

nit: not your fault, but auto now that it's allowed.

[Lei Zhang, 2015/10/07 07:51:12]

another day... maybe dml@ will get to this.

        it != m_pSymbolDictCache->end(); ++it) {
     if (it->first == key) {
       pSegment->m_Result.sd = it->second->DeepCopy();
       m_pSymbolDictCache->push_front(*it);
       m_pSymbolDictCache->erase(it);
       cache_hit = true;
       break;
     }
   }
   if (!cache_hit) {
     if (pSymbolDictDecoder->SDHUFF == 0) {
-      pArithDecoder = new CJBig2_ArithDecoder(m_pStream.get());
-      pSegment->m_Result.sd =
-          pSymbolDictDecoder->decode_Arith(pArithDecoder, gbContext, grContext);
+      CJBig2_ArithDecoder* pArithDecoder =

[Tom Sepez, 2015/10/05 16:34:31]

nit: this could be unique_ptr and lose the delete at 618?

[Lei Zhang, 2015/10/07 07:51:11]

Done.

+          new CJBig2_ArithDecoder(m_pStream.get());
+      pSegment->m_Result.sd = pSymbolDictDecoder->decode_Arith(
+          pArithDecoder, gbContext.get(), grContext.get());
       delete pArithDecoder;
-      if (pSegment->m_Result.sd == NULL) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSegment->m_Result.sd)
+        return JBIG2_ERROR_FATAL;
+
       m_pStream->alignByte();
       m_pStream->offset(2);
     } else {
       pSegment->m_Result.sd = pSymbolDictDecoder->decode_Huffman(
-          m_pStream.get(), gbContext, grContext, pPause);
-      if (pSegment->m_Result.sd == NULL) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+          m_pStream.get(), gbContext.get(), grContext.get(), pPause);
+      if (!pSegment->m_Result.sd)
+        return JBIG2_ERROR_FATAL;
       m_pStream->alignByte();
     }
     CJBig2_SymbolDict* value = pSegment->m_Result.sd->DeepCopy();
     if (value && kSymbolDictCacheMaxSize > 0) {
       while (m_pSymbolDictCache->size() >= kSymbolDictCacheMaxSize) {
         delete m_pSymbolDictCache->back().second;
         m_pSymbolDictCache->pop_back();
       }
       m_pSymbolDictCache->push_front(CJBig2_CachePair(key, value));
     }
   }
   if (wFlags & 0x0200) {
     pSegment->m_Result.sd->m_bContextRetained = TRUE;
     if (pSymbolDictDecoder->SDHUFF == 0) {
-      pSegment->m_Result.sd->m_gbContext = gbContext;
+      pSegment->m_Result.sd->m_gbContext = gbContext.release();
     }
     if (pSymbolDictDecoder->SDREFAGG == 1) {
-      pSegment->m_Result.sd->m_grContext = grContext;
+      pSegment->m_Result.sd->m_grContext = grContext.release();
     }
-    bUsed = TRUE;
-  } else {
-    bUsed = FALSE;
-  }
-  delete pSymbolDictDecoder;
-  FX_Free(SDINSYMS);
-  delete Table_B1;
-  delete Table_B2;
-  delete Table_B3;
-  delete Table_B4;
-  delete Table_B5;
-  if (bUsed == FALSE) {
-    FX_Free(gbContext);
-    FX_Free(grContext);
   }
   return JBIG2_SUCCESS;
-failed:
-  delete pSymbolDictDecoder;
-  FX_Free(SDINSYMS);
-  delete Table_B1;
-  delete Table_B2;
-  delete Table_B3;
-  delete Table_B4;
-  delete Table_B5;
-  FX_Free(gbContext);
-  FX_Free(grContext);
-  return nRet;
 }
 
 int32_t CJBig2_Context::parseTextRegion(CJBig2_Segment* pSegment) {
-  FX_DWORD dwTemp;
   FX_WORD wFlags;
-  int32_t i, nIndex, nRet;
   JBig2RegionInfo ri;
-  CJBig2_Segment* pSeg;
-  CJBig2_Image** SBSYMS = nullptr;
-  JBig2HuffmanCode* SBSYMCODES = nullptr;
-  uint8_t cSBHUFFFS, cSBHUFFDS, cSBHUFFDT, cSBHUFFRDW, cSBHUFFRDH, cSBHUFFRDX,
-      cSBHUFFRDY, cSBHUFFRSIZE;
-  CJBig2_HuffmanTable* Table_B1 = nullptr;
-  CJBig2_HuffmanTable* Table_B6 = nullptr;
-  CJBig2_HuffmanTable* Table_B7 = nullptr;
-  CJBig2_HuffmanTable* Table_B8 = nullptr;
-  CJBig2_HuffmanTable* Table_B9 = nullptr;
-  CJBig2_HuffmanTable* Table_B10 = nullptr;
-  CJBig2_HuffmanTable* Table_B11 = nullptr;
-  CJBig2_HuffmanTable* Table_B12 = nullptr;
-  CJBig2_HuffmanTable* Table_B13 = nullptr;
-  CJBig2_HuffmanTable* Table_B14 = nullptr;
-  CJBig2_HuffmanTable* Table_B15 = nullptr;
-  JBig2ArithCtx* grContext = nullptr;
-  CJBig2_ArithDecoder* pArithDecoder;
-  CJBig2_TRDProc* pTRD = new CJBig2_TRDProc();
   if ((parseRegionInfo(&ri) != JBIG2_SUCCESS) ||

[Tom Sepez, 2015/10/05 16:34:31]

nit: overparenthesized

[Lei Zhang, 2015/10/07 07:51:12]

Done.

       (m_pStream->readShortInteger(&wFlags) != 0)) {
-    nRet = JBIG2_ERROR_TOO_SHORT;
-    goto failed;
+    return JBIG2_ERROR_TOO_SHORT;
   }
+
+  nonstd::unique_ptr<CJBig2_TRDProc> pTRD(new CJBig2_TRDProc);
   pTRD->SBW = ri.width;
   pTRD->SBH = ri.height;
   pTRD->SBHUFF = wFlags & 0x0001;
   pTRD->SBREFINE = (wFlags >> 1) & 0x0001;
-  dwTemp = (wFlags >> 2) & 0x0003;
+  FX_DWORD dwTemp = (wFlags >> 2) & 0x0003;
   pTRD->SBSTRIPS = 1 << dwTemp;
   pTRD->REFCORNER = (JBig2Corner)((wFlags >> 4) & 0x0003);
   pTRD->TRANSPOSED = (wFlags >> 6) & 0x0001;
   pTRD->SBCOMBOP = (JBig2ComposeOp)((wFlags >> 7) & 0x0003);
   pTRD->SBDEFPIXEL = (wFlags >> 9) & 0x0001;
   pTRD->SBDSOFFSET = (wFlags >> 10) & 0x001f;
   if (pTRD->SBDSOFFSET >= 0x0010) {
     pTRD->SBDSOFFSET = pTRD->SBDSOFFSET - 0x0020;
   }
   pTRD->SBRTEMPLATE = (wFlags >> 15) & 0x0001;
+
+  uint8_t cSBHUFFFS;

[Tom Sepez, 2015/10/05 16:34:32]

nit: Yeah, these are used later only when pTRD->SBHUFF, but they sure like
uninitialized.  Hmmm.  Comment?

[Lei Zhang, 2015/10/07 07:51:12]

they are only used when pTRD->SBHUFF == 1.

+  uint8_t cSBHUFFDS;
+  uint8_t cSBHUFFDT;
+  uint8_t cSBHUFFRDW;
+  uint8_t cSBHUFFRDH;
+  uint8_t cSBHUFFRDX;
+  uint8_t cSBHUFFRDY;
+  uint8_t cSBHUFFRSIZE;
   if (pTRD->SBHUFF == 1) {
-    if (m_pStream->readShortInteger(&wFlags) != 0) {
-      nRet = JBIG2_ERROR_TOO_SHORT;
-      goto failed;
-    }
+    if (m_pStream->readShortInteger(&wFlags) != 0)
+      return JBIG2_ERROR_TOO_SHORT;
+
     cSBHUFFFS = wFlags & 0x0003;
     cSBHUFFDS = (wFlags >> 2) & 0x0003;
     cSBHUFFDT = (wFlags >> 4) & 0x0003;
     cSBHUFFRDW = (wFlags >> 6) & 0x0003;
     cSBHUFFRDH = (wFlags >> 8) & 0x0003;
     cSBHUFFRDX = (wFlags >> 10) & 0x0003;
     cSBHUFFRDY = (wFlags >> 12) & 0x0003;
     cSBHUFFRSIZE = (wFlags >> 14) & 0x0001;
   }
   if ((pTRD->SBREFINE == 1) && (pTRD->SBRTEMPLATE == 0)) {
-    for (i = 0; i < 4; i++) {
-      if (m_pStream->read1Byte((uint8_t*)&pTRD->SBRAT[i]) != 0) {
-        nRet = JBIG2_ERROR_TOO_SHORT;
-        goto failed;
-      }
+    for (int32_t i = 0; i < 4; i++) {
+      if (m_pStream->read1Byte((uint8_t*)&pTRD->SBRAT[i]) != 0)
+        return JBIG2_ERROR_TOO_SHORT;
     }
   }
-  if (m_pStream->readInteger(&pTRD->SBNUMINSTANCES) != 0) {
-    nRet = JBIG2_ERROR_TOO_SHORT;
-    goto failed;
+  if (m_pStream->readInteger(&pTRD->SBNUMINSTANCES) != 0)
+    return JBIG2_ERROR_TOO_SHORT;
+
+  for (int32_t i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
+    if (!findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i]))
+      return JBIG2_ERROR_FATAL;
   }
-  for (i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
-    if (!findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i])) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
-  }
+
+  CJBig2_Segment* pSeg;

[Tom Sepez, 2015/10/05 16:34:32]

does this scope tighter even if it costs a couple of parallel decls?

[Lei Zhang, 2015/10/07 07:51:11]

Done.

   pTRD->SBNUMSYMS = 0;
-  for (i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
+  for (int32_t i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
     pSeg = findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i]);
     if (pSeg->m_cFlags.s.type == 0) {
       pTRD->SBNUMSYMS += pSeg->m_Result.sd->SDNUMEXSYMS;
     }
   }
+
+  nonstd::unique_ptr<CJBig2_Image*, FxFreeDeleter> SBSYMS;
   if (pTRD->SBNUMSYMS > 0) {
-    SBSYMS = FX_Alloc(CJBig2_Image*, pTRD->SBNUMSYMS);
+    SBSYMS.reset(FX_Alloc(CJBig2_Image*, pTRD->SBNUMSYMS));
     dwTemp = 0;
-    for (i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
+    for (int32_t i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
       pSeg = findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[i]);
       if (pSeg->m_cFlags.s.type == 0) {
-        JBIG2_memcpy(SBSYMS + dwTemp, pSeg->m_Result.sd->SDEXSYMS,
+        JBIG2_memcpy(SBSYMS.get() + dwTemp, pSeg->m_Result.sd->SDEXSYMS,
                      pSeg->m_Result.sd->SDNUMEXSYMS * sizeof(CJBig2_Image*));
         dwTemp += pSeg->m_Result.sd->SDNUMEXSYMS;
       }
     }
-    pTRD->SBSYMS = SBSYMS;
+    pTRD->SBSYMS = SBSYMS.get();
   } else {
     pTRD->SBSYMS = NULL;
   }
+
+  nonstd::unique_ptr<JBig2HuffmanCode, FxFreeDeleter> SBSYMCODES;
   if (pTRD->SBHUFF == 1) {
-    SBSYMCODES = decodeSymbolIDHuffmanTable(m_pStream.get(), pTRD->SBNUMSYMS);
-    if (SBSYMCODES == NULL) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
+    SBSYMCODES.reset(
+        decodeSymbolIDHuffmanTable(m_pStream.get(), pTRD->SBNUMSYMS));
+    if (!SBSYMCODES)
+      return JBIG2_ERROR_FATAL;
+
     m_pStream->alignByte();
-    pTRD->SBSYMCODES = SBSYMCODES;
+    pTRD->SBSYMCODES = SBSYMCODES.get();
   } else {
     dwTemp = 0;
     while ((FX_DWORD)(1 << dwTemp) < pTRD->SBNUMSYMS) {
       dwTemp++;
     }
     pTRD->SBSYMCODELEN = (uint8_t)dwTemp;
   }
+
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B1;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B6;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B7;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B8;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B9;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B10;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B11;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B12;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B13;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B14;
+  nonstd::unique_ptr<CJBig2_HuffmanTable> Table_B15;
   if (pTRD->SBHUFF == 1) {
     if ((cSBHUFFFS == 2) || (cSBHUFFRDW == 2) || (cSBHUFFRDH == 2) ||
         (cSBHUFFRDX == 2) || (cSBHUFFRDY == 2)) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
+      return JBIG2_ERROR_FATAL;
     }
-    nIndex = 0;
+    int32_t nIndex = 0;
     if (cSBHUFFFS == 0) {
-      Table_B6 = new CJBig2_HuffmanTable(HuffmanTable_B6,
-                                         FX_ArraySize(HuffmanTable_B6),
-                                         HuffmanTable_HTOOB_B6);
-      pTRD->SBHUFFFS = Table_B6;
+      Table_B6.reset(new CJBig2_HuffmanTable(HuffmanTable_B6,
+                                             FX_ArraySize(HuffmanTable_B6),
+                                             HuffmanTable_HTOOB_B6));
+      pTRD->SBHUFFFS = Table_B6.get();
     } else if (cSBHUFFFS == 1) {
-      Table_B7 = new CJBig2_HuffmanTable(HuffmanTable_B7,
-                                         FX_ArraySize(HuffmanTable_B7),
-                                         HuffmanTable_HTOOB_B7);
-      pTRD->SBHUFFFS = Table_B7;
+      Table_B7.reset(new CJBig2_HuffmanTable(HuffmanTable_B7,
+                                             FX_ArraySize(HuffmanTable_B7),
+                                             HuffmanTable_HTOOB_B7));
+      pTRD->SBHUFFFS = Table_B7.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFFS = pSeg->m_Result.ht;
     }
     if (cSBHUFFDS == 0) {
-      Table_B8 = new CJBig2_HuffmanTable(HuffmanTable_B8,
-                                         FX_ArraySize(HuffmanTable_B8),
-                                         HuffmanTable_HTOOB_B8);
-      pTRD->SBHUFFDS = Table_B8;
+      Table_B8.reset(new CJBig2_HuffmanTable(HuffmanTable_B8,
+                                             FX_ArraySize(HuffmanTable_B8),
+                                             HuffmanTable_HTOOB_B8));
+      pTRD->SBHUFFDS = Table_B8.get();
     } else if (cSBHUFFDS == 1) {
-      Table_B9 = new CJBig2_HuffmanTable(HuffmanTable_B9,
-                                         FX_ArraySize(HuffmanTable_B9),
-                                         HuffmanTable_HTOOB_B9);
-      pTRD->SBHUFFDS = Table_B9;
+      Table_B9.reset(new CJBig2_HuffmanTable(HuffmanTable_B9,
+                                             FX_ArraySize(HuffmanTable_B9),
+                                             HuffmanTable_HTOOB_B9));
+      pTRD->SBHUFFDS = Table_B9.get();
     } else if (cSBHUFFDS == 2) {
-      Table_B10 = new CJBig2_HuffmanTable(HuffmanTable_B10,
-                                          FX_ArraySize(HuffmanTable_B10),
-                                          HuffmanTable_HTOOB_B10);
-      pTRD->SBHUFFDS = Table_B10;
+      Table_B10.reset(new CJBig2_HuffmanTable(HuffmanTable_B10,
+                                              FX_ArraySize(HuffmanTable_B10),
+                                              HuffmanTable_HTOOB_B10));
+      pTRD->SBHUFFDS = Table_B10.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFDS = pSeg->m_Result.ht;
     }
     if (cSBHUFFDT == 0) {
-      Table_B11 = new CJBig2_HuffmanTable(HuffmanTable_B11,
-                                          FX_ArraySize(HuffmanTable_B11),
-                                          HuffmanTable_HTOOB_B11);
-      pTRD->SBHUFFDT = Table_B11;
+      Table_B11.reset(new CJBig2_HuffmanTable(HuffmanTable_B11,
+                                              FX_ArraySize(HuffmanTable_B11),
+                                              HuffmanTable_HTOOB_B11));
+      pTRD->SBHUFFDT = Table_B11.get();
     } else if (cSBHUFFDT == 1) {
-      Table_B12 = new CJBig2_HuffmanTable(HuffmanTable_B12,
-                                          FX_ArraySize(HuffmanTable_B12),
-                                          HuffmanTable_HTOOB_B12);
-      pTRD->SBHUFFDT = Table_B12;
+      Table_B12.reset(new CJBig2_HuffmanTable(HuffmanTable_B12,
+                                              FX_ArraySize(HuffmanTable_B12),
+                                              HuffmanTable_HTOOB_B12));
+      pTRD->SBHUFFDT = Table_B12.get();
     } else if (cSBHUFFDT == 2) {
-      Table_B13 = new CJBig2_HuffmanTable(HuffmanTable_B13,
-                                          FX_ArraySize(HuffmanTable_B13),
-                                          HuffmanTable_HTOOB_B13);
-      pTRD->SBHUFFDT = Table_B13;
+      Table_B13.reset(new CJBig2_HuffmanTable(HuffmanTable_B13,
+                                              FX_ArraySize(HuffmanTable_B13),
+                                              HuffmanTable_HTOOB_B13));
+      pTRD->SBHUFFDT = Table_B13.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFDT = pSeg->m_Result.ht;
     }
     if (cSBHUFFRDW == 0) {
-      Table_B14 = new CJBig2_HuffmanTable(HuffmanTable_B14,
-                                          FX_ArraySize(HuffmanTable_B14),
-                                          HuffmanTable_HTOOB_B14);
-      pTRD->SBHUFFRDW = Table_B14;
+      Table_B14.reset(new CJBig2_HuffmanTable(HuffmanTable_B14,
+                                              FX_ArraySize(HuffmanTable_B14),
+                                              HuffmanTable_HTOOB_B14));
+      pTRD->SBHUFFRDW = Table_B14.get();
     } else if (cSBHUFFRDW == 1) {
-      Table_B15 = new CJBig2_HuffmanTable(HuffmanTable_B15,
-                                          FX_ArraySize(HuffmanTable_B15),
-                                          HuffmanTable_HTOOB_B15);
-      pTRD->SBHUFFRDW = Table_B15;
+      Table_B15.reset(new CJBig2_HuffmanTable(HuffmanTable_B15,
+                                              FX_ArraySize(HuffmanTable_B15),
+                                              HuffmanTable_HTOOB_B15));
+      pTRD->SBHUFFRDW = Table_B15.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFRDW = pSeg->m_Result.ht;
     }
     if (cSBHUFFRDH == 0) {
       if (!Table_B14) {
-        Table_B14 = new CJBig2_HuffmanTable(HuffmanTable_B14,
-                                            FX_ArraySize(HuffmanTable_B14),
-                                            HuffmanTable_HTOOB_B14);
+        Table_B14.reset(new CJBig2_HuffmanTable(HuffmanTable_B14,
+                                                FX_ArraySize(HuffmanTable_B14),
+                                                HuffmanTable_HTOOB_B14));
       }
-      pTRD->SBHUFFRDH = Table_B14;
+      pTRD->SBHUFFRDH = Table_B14.get();
     } else if (cSBHUFFRDH == 1) {
       if (!Table_B15) {
-        Table_B15 = new CJBig2_HuffmanTable(HuffmanTable_B15,
-                                            FX_ArraySize(HuffmanTable_B15),
-                                            HuffmanTable_HTOOB_B15);
+        Table_B15.reset(new CJBig2_HuffmanTable(HuffmanTable_B15,
+                                                FX_ArraySize(HuffmanTable_B15),
+                                                HuffmanTable_HTOOB_B15));
       }
-      pTRD->SBHUFFRDH = Table_B15;
+      pTRD->SBHUFFRDH = Table_B15.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFRDH = pSeg->m_Result.ht;
     }
     if (cSBHUFFRDX == 0) {
       if (!Table_B14) {
-        Table_B14 = new CJBig2_HuffmanTable(HuffmanTable_B14,
-                                            FX_ArraySize(HuffmanTable_B14),
-                                            HuffmanTable_HTOOB_B14);
+        Table_B14.reset(new CJBig2_HuffmanTable(HuffmanTable_B14,
+                                                FX_ArraySize(HuffmanTable_B14),
+                                                HuffmanTable_HTOOB_B14));
       }
-      pTRD->SBHUFFRDX = Table_B14;
+      pTRD->SBHUFFRDX = Table_B14.get();
     } else if (cSBHUFFRDX == 1) {
       if (!Table_B15) {
-        Table_B15 = new CJBig2_HuffmanTable(HuffmanTable_B15,
-                                            FX_ArraySize(HuffmanTable_B15),
-                                            HuffmanTable_HTOOB_B15);
+        Table_B15.reset(new CJBig2_HuffmanTable(HuffmanTable_B15,
+                                                FX_ArraySize(HuffmanTable_B15),
+                                                HuffmanTable_HTOOB_B15));
       }
-      pTRD->SBHUFFRDX = Table_B15;
+      pTRD->SBHUFFRDX = Table_B15.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFRDX = pSeg->m_Result.ht;
     }
     if (cSBHUFFRDY == 0) {
       if (!Table_B14) {
-        Table_B14 = new CJBig2_HuffmanTable(HuffmanTable_B14,
-                                            FX_ArraySize(HuffmanTable_B14),
-                                            HuffmanTable_HTOOB_B14);
+        Table_B14.reset(new CJBig2_HuffmanTable(HuffmanTable_B14,
+                                                FX_ArraySize(HuffmanTable_B14),
+                                                HuffmanTable_HTOOB_B14));
       }
-      pTRD->SBHUFFRDY = Table_B14;
+      pTRD->SBHUFFRDY = Table_B14.get();
     } else if (cSBHUFFRDY == 1) {
       if (!Table_B15) {
-        Table_B15 = new CJBig2_HuffmanTable(HuffmanTable_B15,
-                                            FX_ArraySize(HuffmanTable_B15),
-                                            HuffmanTable_HTOOB_B15);
+        Table_B15.reset(new CJBig2_HuffmanTable(HuffmanTable_B15,
+                                                FX_ArraySize(HuffmanTable_B15),
+                                                HuffmanTable_HTOOB_B15));
       }
-      pTRD->SBHUFFRDY = Table_B15;
+      pTRD->SBHUFFRDY = Table_B15.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFRDY = pSeg->m_Result.ht;
     }
     if (cSBHUFFRSIZE == 0) {
-      Table_B1 = new CJBig2_HuffmanTable(HuffmanTable_B1,
-                                         FX_ArraySize(HuffmanTable_B1),
-                                         HuffmanTable_HTOOB_B1);
-      pTRD->SBHUFFRSIZE = Table_B1;
+      Table_B1.reset(new CJBig2_HuffmanTable(HuffmanTable_B1,
+                                             FX_ArraySize(HuffmanTable_B1),
+                                             HuffmanTable_HTOOB_B1));
+      pTRD->SBHUFFRSIZE = Table_B1.get();
     } else {
       pSeg = findReferredSegmentByTypeAndIndex(pSegment, 53, nIndex++);
-      if (!pSeg) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
       pTRD->SBHUFFRSIZE = pSeg->m_Result.ht;
     }
   }
+  nonstd::unique_ptr<JBig2ArithCtx, FxFreeDeleter> grContext;
   if (pTRD->SBREFINE == 1) {
     dwTemp = pTRD->SBRTEMPLATE ? 1 << 10 : 1 << 13;
-    grContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-    JBIG2_memset(grContext, 0, sizeof(JBig2ArithCtx) * dwTemp);
+    grContext.reset(FX_Alloc(JBig2ArithCtx, dwTemp));
+    JBIG2_memset(grContext.get(), 0, sizeof(JBig2ArithCtx) * dwTemp);
   }
   if (pTRD->SBHUFF == 0) {
-    pArithDecoder = new CJBig2_ArithDecoder(m_pStream.get());
+    CJBig2_ArithDecoder* pArithDecoder =
+        new CJBig2_ArithDecoder(m_pStream.get());
     pSegment->m_nResultType = JBIG2_IMAGE_POINTER;
-    pSegment->m_Result.im = pTRD->decode_Arith(pArithDecoder, grContext);
+    pSegment->m_Result.im = pTRD->decode_Arith(pArithDecoder, grContext.get());
     delete pArithDecoder;
-    if (pSegment->m_Result.im == NULL) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
+    if (!pSegment->m_Result.im)
+      return JBIG2_ERROR_FATAL;
     m_pStream->alignByte();
     m_pStream->offset(2);
   } else {
     pSegment->m_nResultType = JBIG2_IMAGE_POINTER;
-    pSegment->m_Result.im = pTRD->decode_Huffman(m_pStream.get(), grContext);
-    if (pSegment->m_Result.im == NULL) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
+    pSegment->m_Result.im =
+        pTRD->decode_Huffman(m_pStream.get(), grContext.get());
+    if (!pSegment->m_Result.im)
+      return JBIG2_ERROR_FATAL;
     m_pStream->alignByte();
   }
   if (pSegment->m_cFlags.s.type != 4) {
     if (!m_bBufSpecified) {
       JBig2PageInfo* pPageInfo = m_PageInfoList.back();
       if ((pPageInfo->m_bIsStriped == 1) &&
           (ri.y + ri.height > m_pPage->m_nHeight)) {
         m_pPage->expand(ri.y + ri.height, (pPageInfo->m_cFlags & 4) ? 1 : 0);
       }
     }
     m_pPage->composeFrom(ri.x, ri.y, pSegment->m_Result.im,
                          (JBig2ComposeOp)(ri.flags & 0x03));
     delete pSegment->m_Result.im;
     pSegment->m_Result.im = NULL;
   }
-  delete pTRD;
-  FX_Free(SBSYMS);
-  FX_Free(SBSYMCODES);
-  FX_Free(grContext);
-  delete Table_B1;
-  delete Table_B6;
-  delete Table_B7;
-  delete Table_B8;
-  delete Table_B9;
-  delete Table_B10;
-  delete Table_B11;
-  delete Table_B12;
-  delete Table_B13;
-  delete Table_B14;
-  delete Table_B15;
   return JBIG2_SUCCESS;
-failed:
-  delete pTRD;
-  FX_Free(SBSYMS);
-  FX_Free(SBSYMCODES);
-  FX_Free(grContext);
-  delete Table_B1;
-  delete Table_B6;
-  delete Table_B7;
-  delete Table_B8;
-  delete Table_B9;
-  delete Table_B10;
-  delete Table_B11;
-  delete Table_B12;
-  delete Table_B13;
-  delete Table_B14;
-  delete Table_B15;
-  return nRet;
 }
 
 int32_t CJBig2_Context::parsePatternDict(CJBig2_Segment* pSegment,
                                          IFX_Pause* pPause) {
-  FX_DWORD dwTemp;
   uint8_t cFlags;
-  JBig2ArithCtx* gbContext;
-  CJBig2_ArithDecoder* pArithDecoder;
-  int32_t nRet;
-  CJBig2_PDDProc* pPDD = new CJBig2_PDDProc();
+  nonstd::unique_ptr<CJBig2_PDDProc> pPDD(new CJBig2_PDDProc);
   if ((m_pStream->read1Byte(&cFlags) != 0) ||
       (m_pStream->read1Byte(&pPDD->HDPW) != 0) ||
       (m_pStream->read1Byte(&pPDD->HDPH) != 0) ||
       (m_pStream->readInteger(&pPDD->GRAYMAX) != 0)) {
-    nRet = JBIG2_ERROR_TOO_SHORT;
-    goto failed;
+    return JBIG2_ERROR_TOO_SHORT;
   }
-  if (pPDD->GRAYMAX > JBIG2_MAX_PATTERN_INDEX) {
-    nRet = JBIG2_ERROR_LIMIT;
-    goto failed;
-  }
+  if (pPDD->GRAYMAX > JBIG2_MAX_PATTERN_INDEX)
+    return JBIG2_ERROR_LIMIT;
+
   pPDD->HDMMR = cFlags & 0x01;
   pPDD->HDTEMPLATE = (cFlags >> 1) & 0x03;
   pSegment->m_nResultType = JBIG2_PATTERN_DICT_POINTER;
   if (pPDD->HDMMR == 0) {
-    dwTemp =
+    FX_DWORD dwTemp =
         pPDD->HDTEMPLATE == 0 ? 65536 : pPDD->HDTEMPLATE == 1 ? 8192 : 1024;
-    gbContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-    JBIG2_memset(gbContext, 0, sizeof(JBig2ArithCtx) * dwTemp);
-    pArithDecoder = new CJBig2_ArithDecoder(m_pStream.get());
+    nonstd::unique_ptr<JBig2ArithCtx, FxFreeDeleter> gbContext(
+        FX_Alloc(JBig2ArithCtx, dwTemp));
+    JBIG2_memset(gbContext.get(), 0, sizeof(JBig2ArithCtx) * dwTemp);
+    CJBig2_ArithDecoder* pArithDecoder =

[Tom Sepez, 2015/10/05 16:34:32]

nit: unique_ptr here, too?

[Lei Zhang, 2015/10/07 07:51:11]

Done.

+        new CJBig2_ArithDecoder(m_pStream.get());
     pSegment->m_Result.pd =
-        pPDD->decode_Arith(pArithDecoder, gbContext, pPause);
+        pPDD->decode_Arith(pArithDecoder, gbContext.get(), pPause);
     delete pArithDecoder;
-    if (pSegment->m_Result.pd == NULL) {
-      FX_Free(gbContext);
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
-    FX_Free(gbContext);
+    if (!pSegment->m_Result.pd)
+      return JBIG2_ERROR_FATAL;
+
     m_pStream->alignByte();
     m_pStream->offset(2);
   } else {
     pSegment->m_Result.pd = pPDD->decode_MMR(m_pStream.get(), pPause);
-    if (pSegment->m_Result.pd == NULL) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
+    if (!pSegment->m_Result.pd)
+      return JBIG2_ERROR_FATAL;
     m_pStream->alignByte();
   }
-  delete pPDD;
   return JBIG2_SUCCESS;
-failed:
-  delete pPDD;
-  return nRet;
 }
+
 int32_t CJBig2_Context::parseHalftoneRegion(CJBig2_Segment* pSegment,
                                             IFX_Pause* pPause) {
-  FX_DWORD dwTemp;
   uint8_t cFlags;
   JBig2RegionInfo ri;
-  CJBig2_Segment* pSeg;
-  CJBig2_PatternDict* pPatternDict;
-  JBig2ArithCtx* gbContext;
-  CJBig2_ArithDecoder* pArithDecoder;
-  int32_t nRet;
-  CJBig2_HTRDProc* pHRD = new CJBig2_HTRDProc();
+  nonstd::unique_ptr<CJBig2_HTRDProc> pHRD(new CJBig2_HTRDProc);
   if ((parseRegionInfo(&ri) != JBIG2_SUCCESS) ||
       (m_pStream->read1Byte(&cFlags) != 0) ||
       (m_pStream->readInteger(&pHRD->HGW) != 0) ||
       (m_pStream->readInteger(&pHRD->HGH) != 0) ||
       (m_pStream->readInteger((FX_DWORD*)&pHRD->HGX) != 0) ||

[Tom Sepez, 2015/10/05 16:34:31]

nit: theoretically illegal casts here. 

[Lei Zhang, 2015/10/07 07:51:12]

file a bug so I don't forget these?

       (m_pStream->readInteger((FX_DWORD*)&pHRD->HGY) != 0) ||
       (m_pStream->readShortInteger(&pHRD->HRX) != 0) ||
       (m_pStream->readShortInteger(&pHRD->HRY) != 0)) {
-    nRet = JBIG2_ERROR_TOO_SHORT;
-    goto failed;
+    return JBIG2_ERROR_TOO_SHORT;
   }
-  if (pHRD->HGW == 0 || pHRD->HGH == 0) {
-    nRet = JBIG2_ERROR_FATAL;
-    goto failed;
-  }
+
+  if (pHRD->HGW == 0 || pHRD->HGH == 0)
+    return JBIG2_ERROR_FATAL;
 
   pHRD->HBW = ri.width;
   pHRD->HBH = ri.height;
   pHRD->HMMR = cFlags & 0x01;
   pHRD->HTEMPLATE = (cFlags >> 1) & 0x03;
   pHRD->HENABLESKIP = (cFlags >> 3) & 0x01;
   pHRD->HCOMBOP = (JBig2ComposeOp)((cFlags >> 4) & 0x07);
   pHRD->HDEFPIXEL = (cFlags >> 7) & 0x01;
-  if (pSegment->m_nReferred_to_segment_count != 1) {
-    nRet = JBIG2_ERROR_FATAL;
-    goto failed;
-  }
-  pSeg = findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[0]);
-  if ((pSeg == NULL) || (pSeg->m_cFlags.s.type != 16)) {
-    nRet = JBIG2_ERROR_FATAL;
-    goto failed;
-  }
-  pPatternDict = pSeg->m_Result.pd;
-  if ((pPatternDict == NULL) || (pPatternDict->NUMPATS == 0)) {
-    nRet = JBIG2_ERROR_FATAL;
-    goto failed;
-  }
+  if (pSegment->m_nReferred_to_segment_count != 1)
+    return JBIG2_ERROR_FATAL;
+
+  CJBig2_Segment* pSeg =
+      findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[0]);
+  if (!pSeg || (pSeg->m_cFlags.s.type != 16))
+    return JBIG2_ERROR_FATAL;

[Tom Sepez, 2015/10/05 16:34:32]

Does this leak for the non-16 case?

[Lei Zhang, 2015/10/07 07:51:11]

What are we leaking? |pSeg|? I don't think we have ownership here.

[Tom Sepez, 2015/10/07 17:13:12]

Acknowledged.

+
+  CJBig2_PatternDict* pPatternDict = pSeg->m_Result.pd;
+  if (!pPatternDict || (pPatternDict->NUMPATS == 0))
+    return JBIG2_ERROR_FATAL;

[Tom Sepez, 2015/10/05 16:34:32]

ditto

[Lei Zhang, 2015/10/07 07:51:12]

Ditto, no leak?

[Tom Sepez, 2015/10/07 17:13:12]

Acknowledged.

+
   pHRD->HNUMPATS = pPatternDict->NUMPATS;
   pHRD->HPATS = pPatternDict->HDPATS;
   pHRD->HPW = pPatternDict->HDPATS[0]->m_nWidth;
   pHRD->HPH = pPatternDict->HDPATS[0]->m_nHeight;
   pSegment->m_nResultType = JBIG2_IMAGE_POINTER;
   if (pHRD->HMMR == 0) {
-    dwTemp = pHRD->HTEMPLATE == 0 ? 65536 : pHRD->HTEMPLATE == 1 ? 8192 : 1024;
-    gbContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-    JBIG2_memset(gbContext, 0, sizeof(JBig2ArithCtx) * dwTemp);
-    pArithDecoder = new CJBig2_ArithDecoder(m_pStream.get());
+    FX_DWORD dwTemp =
+        pHRD->HTEMPLATE == 0 ? 65536 : pHRD->HTEMPLATE == 1 ? 8192 : 1024;
+    nonstd::unique_ptr<JBig2ArithCtx, FxFreeDeleter> gbContext(
+        FX_Alloc(JBig2ArithCtx, dwTemp));
+    JBIG2_memset(gbContext.get(), 0, sizeof(JBig2ArithCtx) * dwTemp);
+    CJBig2_ArithDecoder* pArithDecoder =

[Tom Sepez, 2015/10/05 16:34:31]

nonstd unique ptr?

[Lei Zhang, 2015/10/07 07:51:11]

Done.

+        new CJBig2_ArithDecoder(m_pStream.get());
     pSegment->m_Result.im =
-        pHRD->decode_Arith(pArithDecoder, gbContext, pPause);
+        pHRD->decode_Arith(pArithDecoder, gbContext.get(), pPause);
     delete pArithDecoder;
-    if (pSegment->m_Result.im == NULL) {
-      FX_Free(gbContext);
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
-    FX_Free(gbContext);
+    if (!pSegment->m_Result.im)
+      return JBIG2_ERROR_FATAL;
+
     m_pStream->alignByte();
     m_pStream->offset(2);
   } else {
     pSegment->m_Result.im = pHRD->decode_MMR(m_pStream.get(), pPause);
-    if (pSegment->m_Result.im == NULL) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
+    if (!pSegment->m_Result.im)
+      return JBIG2_ERROR_FATAL;
     m_pStream->alignByte();
   }
   if (pSegment->m_cFlags.s.type != 20) {
     if (!m_bBufSpecified) {
       JBig2PageInfo* pPageInfo = m_PageInfoList.back();
       if ((pPageInfo->m_bIsStriped == 1) &&
           (ri.y + ri.height > m_pPage->m_nHeight)) {
         m_pPage->expand(ri.y + ri.height, (pPageInfo->m_cFlags & 4) ? 1 : 0);
       }
     }
     m_pPage->composeFrom(ri.x, ri.y, pSegment->m_Result.im,
                          (JBig2ComposeOp)(ri.flags & 0x03));
     delete pSegment->m_Result.im;
     pSegment->m_Result.im = NULL;
   }
-  delete pHRD;
   return JBIG2_SUCCESS;
-failed:
-  delete pHRD;
-  return nRet;
 }
 
 int32_t CJBig2_Context::parseGenericRegion(CJBig2_Segment* pSegment,
                                            IFX_Pause* pPause) {
-  FX_DWORD dwTemp;
-  uint8_t cFlags;
-  int32_t i, nRet;
   if (!m_pGRD) {
     m_pGRD.reset(new CJBig2_GRDProc);

[Tom Sepez, 2015/10/05 16:34:31]

maybe a local unique_ptr and nonstd::move() at the end of the block avoids a lot
of these resets to null.

[Lei Zhang, 2015/10/07 07:51:11]

Done.

+    uint8_t cFlags;
     if ((parseRegionInfo(&m_ri) != JBIG2_SUCCESS) ||
         (m_pStream->read1Byte(&cFlags) != 0)) {
-      nRet = JBIG2_ERROR_TOO_SHORT;
-      goto failed;
+      m_pGRD.reset();
+      return JBIG2_ERROR_TOO_SHORT;
     }
     if (m_ri.height < 0 || m_ri.width < 0) {
-      nRet = JBIG2_FAILED;
-      goto failed;
+      m_pGRD.reset();
+      return JBIG2_FAILED;
     }
     m_pGRD->GBW = m_ri.width;
     m_pGRD->GBH = m_ri.height;
     m_pGRD->MMR = cFlags & 0x01;
     m_pGRD->GBTEMPLATE = (cFlags >> 1) & 0x03;
     m_pGRD->TPGDON = (cFlags >> 3) & 0x01;
     if (m_pGRD->MMR == 0) {
       if (m_pGRD->GBTEMPLATE == 0) {
-        for (i = 0; i < 8; i++) {
+        for (int32_t i = 0; i < 8; i++) {
           if (m_pStream->read1Byte((uint8_t*)&m_pGRD->GBAT[i]) != 0) {
-            nRet = JBIG2_ERROR_TOO_SHORT;
-            goto failed;
+            m_pGRD.reset();
+            return JBIG2_ERROR_TOO_SHORT;
           }
         }
       } else {
-        for (i = 0; i < 2; i++) {
+        for (int32_t i = 0; i < 2; i++) {
           if (m_pStream->read1Byte((uint8_t*)&m_pGRD->GBAT[i]) != 0) {
-            nRet = JBIG2_ERROR_TOO_SHORT;
-            goto failed;
+            m_pGRD.reset();
+            return JBIG2_ERROR_TOO_SHORT;
           }
         }
       }
     }
     m_pGRD->USESKIP = 0;
   }
   pSegment->m_nResultType = JBIG2_IMAGE_POINTER;
   if (m_pGRD->MMR == 0) {
-    dwTemp =
+    FX_DWORD dwTemp =
         m_pGRD->GBTEMPLATE == 0 ? 65536 : m_pGRD->GBTEMPLATE == 1 ? 8192 : 1024;
-    if (m_gbContext == NULL) {
+    if (!m_gbContext) {
       m_gbContext = FX_Alloc(JBig2ArithCtx, dwTemp);
       JBIG2_memset(m_gbContext, 0, sizeof(JBig2ArithCtx) * dwTemp);
     }
-    if (m_pArithDecoder == NULL) {
+    if (!m_pArithDecoder) {
       m_pArithDecoder = new CJBig2_ArithDecoder(m_pStream.get());
       m_ProcessingStatus = m_pGRD->Start_decode_Arith(
           &pSegment->m_Result.im, m_pArithDecoder, m_gbContext, pPause);
     } else {
       m_ProcessingStatus = m_pGRD->Continue_decode(pPause);
     }
     if (m_ProcessingStatus == FXCODEC_STATUS_DECODE_TOBECONTINUE) {
       if (pSegment->m_cFlags.s.type != 36) {
         if (!m_bBufSpecified) {
           JBig2PageInfo* pPageInfo = m_PageInfoList.back();
           if ((pPageInfo->m_bIsStriped == 1) &&
               (m_ri.y + m_ri.height > m_pPage->m_nHeight)) {
             m_pPage->expand(m_ri.y + m_ri.height,
                             (pPageInfo->m_cFlags & 4) ? 1 : 0);
           }
         }
         FX_RECT Rect = m_pGRD->GetReplaceRect();
         m_pPage->composeFrom(m_ri.x + Rect.left, m_ri.y + Rect.top,
                              pSegment->m_Result.im,
                              (JBig2ComposeOp)(m_ri.flags & 0x03), &Rect);
       }
       return JBIG2_SUCCESS;
     } else {
       delete m_pArithDecoder;

[Tom Sepez, 2015/10/05 16:34:31]

should m_pArithDecoder be unique_ptr?

[Lei Zhang, 2015/10/07 07:51:12]

Done.

       m_pArithDecoder = NULL;
-      if (pSegment->m_Result.im == NULL) {
-        FX_Free(m_gbContext);
-        nRet = JBIG2_ERROR_FATAL;
-        m_gbContext = NULL;
-        m_ProcessingStatus = FXCODEC_STATUS_ERROR;
-        goto failed;
-      }
       FX_Free(m_gbContext);
       m_gbContext = NULL;
+      if (!pSegment->m_Result.im) {
+        m_ProcessingStatus = FXCODEC_STATUS_ERROR;
+        m_pGRD.reset();
+        return JBIG2_ERROR_FATAL;
+      }
       m_pStream->alignByte();
       m_pStream->offset(2);
     }
   } else {
     FXCODEC_STATUS status = m_pGRD->Start_decode_MMR(&pSegment->m_Result.im,
                                                      m_pStream.get(), pPause);
     while (status == FXCODEC_STATUS_DECODE_TOBECONTINUE) {
       m_pGRD->Continue_decode(pPause);
     }
-    if (pSegment->m_Result.im == NULL) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
+    if (!pSegment->m_Result.im) {
+      m_pGRD.reset();
+      return JBIG2_ERROR_FATAL;
     }
     m_pStream->alignByte();
   }
   if (pSegment->m_cFlags.s.type != 36) {
     if (!m_bBufSpecified) {
       JBig2PageInfo* pPageInfo = m_PageInfoList.back();
       if ((pPageInfo->m_bIsStriped == 1) &&
           (m_ri.y + m_ri.height > m_pPage->m_nHeight)) {
         m_pPage->expand(m_ri.y + m_ri.height,
                         (pPageInfo->m_cFlags & 4) ? 1 : 0);
       }
     }
     FX_RECT Rect = m_pGRD->GetReplaceRect();
     m_pPage->composeFrom(m_ri.x + Rect.left, m_ri.y + Rect.top,
                          pSegment->m_Result.im,
                          (JBig2ComposeOp)(m_ri.flags & 0x03), &Rect);
     delete pSegment->m_Result.im;
     pSegment->m_Result.im = NULL;
   }
   m_pGRD.reset();
   return JBIG2_SUCCESS;
-failed:
-  m_pGRD.reset();
-  return nRet;
 }
 
 int32_t CJBig2_Context::parseGenericRefinementRegion(CJBig2_Segment* pSegment) {
-  FX_DWORD dwTemp;
   JBig2RegionInfo ri;
-  CJBig2_Segment* pSeg;
-  int32_t i, nRet;
   uint8_t cFlags;
-  JBig2ArithCtx* grContext;
-  CJBig2_ArithDecoder* pArithDecoder;
-  CJBig2_GRRDProc* pGRRD = new CJBig2_GRRDProc();
   if ((parseRegionInfo(&ri) != JBIG2_SUCCESS) ||
       (m_pStream->read1Byte(&cFlags) != 0)) {
-    nRet = JBIG2_ERROR_TOO_SHORT;
-    goto failed;
+    return JBIG2_ERROR_TOO_SHORT;
   }
+  nonstd::unique_ptr<CJBig2_GRRDProc> pGRRD(new CJBig2_GRRDProc);
   pGRRD->GRW = ri.width;
   pGRRD->GRH = ri.height;
   pGRRD->GRTEMPLATE = cFlags & 0x01;
   pGRRD->TPGRON = (cFlags >> 1) & 0x01;
   if (pGRRD->GRTEMPLATE == 0) {
-    for (i = 0; i < 4; i++) {
-      if (m_pStream->read1Byte((uint8_t*)&pGRRD->GRAT[i]) != 0) {
-        nRet = JBIG2_ERROR_TOO_SHORT;
-        goto failed;
-      }
+    for (int32_t i = 0; i < 4; i++) {
+      if (m_pStream->read1Byte((uint8_t*)&pGRRD->GRAT[i]) != 0)
+        return JBIG2_ERROR_TOO_SHORT;
     }
   }
-  pSeg = NULL;
+  CJBig2_Segment* pSeg = nullptr;
   if (pSegment->m_nReferred_to_segment_count > 0) {
+    int32_t i;
     for (i = 0; i < pSegment->m_nReferred_to_segment_count; i++) {
       pSeg = findSegmentByNumber(pSegment->m_pReferred_to_segment_numbers[0]);
-      if (pSeg == NULL) {
-        nRet = JBIG2_ERROR_FATAL;
-        goto failed;
-      }
+      if (!pSeg)
+        return JBIG2_ERROR_FATAL;
+
       if ((pSeg->m_cFlags.s.type == 4) || (pSeg->m_cFlags.s.type == 20) ||
           (pSeg->m_cFlags.s.type == 36) || (pSeg->m_cFlags.s.type == 40)) {
         break;
       }
     }
-    if (i >= pSegment->m_nReferred_to_segment_count) {
-      nRet = JBIG2_ERROR_FATAL;
-      goto failed;
-    }
+    if (i >= pSegment->m_nReferred_to_segment_count)
+      return JBIG2_ERROR_FATAL;
+
     pGRRD->GRREFERENCE = pSeg->m_Result.im;
   } else {
     pGRRD->GRREFERENCE = m_pPage.get();
   }
   pGRRD->GRREFERENCEDX = 0;
   pGRRD->GRREFERENCEDY = 0;
-  dwTemp = pGRRD->GRTEMPLATE ? 1 << 10 : 1 << 13;
-  grContext = FX_Alloc(JBig2ArithCtx, dwTemp);
-  JBIG2_memset(grContext, 0, sizeof(JBig2ArithCtx) * dwTemp);
-  pArithDecoder = new CJBig2_ArithDecoder(m_pStream.get());
+  FX_DWORD dwTemp = pGRRD->GRTEMPLATE ? 1 << 10 : 1 << 13;

[Tom Sepez, 2015/10/05 16:34:32]

nit: our friends 1k and 8k again.

[Lei Zhang, 2015/10/07 07:51:12]

Done.

+  nonstd::unique_ptr<JBig2ArithCtx, FxFreeDeleter> grContext(
+      FX_Alloc(JBig2ArithCtx, dwTemp));
+  JBIG2_memset(grContext.get(), 0, sizeof(JBig2ArithCtx) * dwTemp);
+  CJBig2_ArithDecoder* pArithDecoder = new CJBig2_ArithDecoder(m_pStream.get());
   pSegment->m_nResultType = JBIG2_IMAGE_POINTER;
-  pSegment->m_Result.im = pGRRD->decode(pArithDecoder, grContext);
+  pSegment->m_Result.im = pGRRD->decode(pArithDecoder, grContext.get());
   delete pArithDecoder;

[Tom Sepez, 2015/10/05 16:34:32]

same pattern here as above 

[Lei Zhang, 2015/10/07 07:51:11]

Done.

-  if (pSegment->m_Result.im == NULL) {
-    FX_Free(grContext);
-    nRet = JBIG2_ERROR_FATAL;
-    goto failed;
-  }
-  FX_Free(grContext);
+  if (!pSegment->m_Result.im)
+    return JBIG2_ERROR_FATAL;
+
   m_pStream->alignByte();
   m_pStream->offset(2);
   if (pSegment->m_cFlags.s.type != 40) {
     if (!m_bBufSpecified) {
       JBig2PageInfo* pPageInfo = m_PageInfoList.back();
       if ((pPageInfo->m_bIsStriped == 1) &&
           (ri.y + ri.height > m_pPage->m_nHeight)) {
         m_pPage->expand(ri.y + ri.height, (pPageInfo->m_cFlags & 4) ? 1 : 0);
       }
     }
     m_pPage->composeFrom(ri.x, ri.y, pSegment->m_Result.im,
                          (JBig2ComposeOp)(ri.flags & 0x03));
     delete pSegment->m_Result.im;
     pSegment->m_Result.im = NULL;
   }
-  delete pGRRD;
   return JBIG2_SUCCESS;
-failed:
-  delete pGRRD;
-  return nRet;
 }
+
 int32_t CJBig2_Context::parseTable(CJBig2_Segment* pSegment) {
   pSegment->m_nResultType = JBIG2_HUFFMAN_TABLE_POINTER;
   pSegment->m_Result.ht = new CJBig2_HuffmanTable(m_pStream.get());
   if (!pSegment->m_Result.ht->isOK()) {
     delete pSegment->m_Result.ht;
     pSegment->m_Result.ht = NULL;
     return JBIG2_ERROR_FATAL;
   }
   m_pStream->alignByte();
   return JBIG2_SUCCESS;
 }
+
 int32_t CJBig2_Context::parseRegionInfo(JBig2RegionInfo* pRI) {
   if ((m_pStream->readInteger((FX_DWORD*)&pRI->width) != 0) ||
       (m_pStream->readInteger((FX_DWORD*)&pRI->height) != 0) ||
       (m_pStream->readInteger((FX_DWORD*)&pRI->x) != 0) ||
       (m_pStream->readInteger((FX_DWORD*)&pRI->y) != 0) ||
       (m_pStream->read1Byte(&pRI->flags) != 0)) {
     return JBIG2_ERROR_TOO_SHORT;
   }
   return JBIG2_SUCCESS;
 }
+
 JBig2HuffmanCode* CJBig2_Context::decodeSymbolIDHuffmanTable(
     CJBig2_BitStream* pStream,
     FX_DWORD SBNUMSYMS) {
-  JBig2HuffmanCode* SBSYMCODES;
-  int32_t runcodes[35];
-  int32_t runcodes_len[35];
-  int32_t runcode;
-  int32_t i;
-  int32_t j;
-  int32_t nVal;
-  int32_t nBits;
+  size_t kRunCodesSize = 35;
+  int32_t runcodes[kRunCodesSize];
+  int32_t runcodes_len[kRunCodesSize];
+  for (int32_t i = 0; i < kRunCodesSize; i++) {
+    if (pStream->readNBits(4, &runcodes_len[i]) != 0)
+      return nullptr;
+  }
+  huffman_assign_code(runcodes, runcodes_len, kRunCodesSize);
+
+  nonstd::unique_ptr<JBig2HuffmanCode, FxFreeDeleter> SBSYMCODES(
+      FX_Alloc(JBig2HuffmanCode, SBNUMSYMS));
   int32_t run;
-  FX_DWORD nTemp;
-  SBSYMCODES = FX_Alloc(JBig2HuffmanCode, SBNUMSYMS);
-  for (i = 0; i < 35; i++) {
-    if (pStream->readNBits(4, &runcodes_len[i]) != 0) {
-      goto failed;
-    }
-  }
-  huffman_assign_code(runcodes, runcodes_len, 35);
-  i = 0;
+  int32_t i = 0;
   while (i < (int)SBNUMSYMS) {
-    nVal = 0;
-    nBits = 0;
-    for (;;) {
-      if (pStream->read1Bit(&nTemp) != 0) {
-        goto failed;
-      }
+    int32_t j;
+    int32_t nVal = 0;
+    int32_t nBits = 0;
+    FX_DWORD nTemp;
+    while (true) {
+      if (pStream->read1Bit(&nTemp) != 0)
+        return nullptr;
+
       nVal = (nVal << 1) | nTemp;
       nBits++;
-      for (j = 0; j < 35; j++) {
+      for (j = 0; j < kRunCodesSize; j++) {
         if ((nBits == runcodes_len[j]) && (nVal == runcodes[j])) {

[Tom Sepez, 2015/10/05 16:34:31]

nit: overparenthesized.

[Lei Zhang, 2015/10/07 07:51:11]

Done.

           break;
         }
       }
-      if (j < 35) {
+      if (j < kRunCodesSize) {
         break;
       }
     }
-    runcode = j;
+    int32_t runcode = j;
     if (runcode < 32) {
-      SBSYMCODES[i].codelen = runcode;
+      SBSYMCODES.get()[i].codelen = runcode;
       run = 0;
     } else if (runcode == 32) {
-      if (pStream->readNBits(2, &nTemp) != 0) {
-        goto failed;
-      }
+      if (pStream->readNBits(2, &nTemp) != 0)
+        return nullptr;
       run = nTemp + 3;
     } else if (runcode == 33) {
-      if (pStream->readNBits(3, &nTemp) != 0) {
-        goto failed;
-      }
+      if (pStream->readNBits(3, &nTemp) != 0)
+        return nullptr;
       run = nTemp + 3;
     } else if (runcode == 34) {
-      if (pStream->readNBits(7, &nTemp) != 0) {
-        goto failed;
-      }
+      if (pStream->readNBits(7, &nTemp) != 0)
+        return nullptr;
       run = nTemp + 11;
     }
     if (run > 0) {
-      if (i + run > (int)SBNUMSYMS) {
-        goto failed;
-      }
+      if (i + run > (int)SBNUMSYMS)
+        return nullptr;
       for (j = 0; j < run; j++) {
         if (runcode == 32 && i > 0) {
-          SBSYMCODES[i + j].codelen = SBSYMCODES[i - 1].codelen;
+          SBSYMCODES.get()[i + j].codelen = SBSYMCODES.get()[i - 1].codelen;
         } else {
-          SBSYMCODES[i + j].codelen = 0;
+          SBSYMCODES.get()[i + j].codelen = 0;
         }
       }
       i += run;
     } else {
       i++;
     }
   }
-  huffman_assign_code(SBSYMCODES, SBNUMSYMS);
-  return SBSYMCODES;
-failed:
-  FX_Free(SBSYMCODES);
-  return NULL;
+  huffman_assign_code(SBSYMCODES.get(), SBNUMSYMS);
+  return SBSYMCODES.release();
 }
+
 void CJBig2_Context::huffman_assign_code(int* CODES, int* PREFLEN, int NTEMP) {
   int CURLEN, LENMAX, CURCODE, CURTEMP, i;
   int* LENCOUNT;
   int* FIRSTCODE;
   LENMAX = 0;
   for (i = 0; i < NTEMP; i++) {
     if (PREFLEN[i] > LENMAX) {
       LENMAX = PREFLEN[i];
     }
   }
@@ skipping 51 matching lines @@
         SBSYMCODES[CURTEMP].code = CURCODE;
         CURCODE = CURCODE + 1;
       }
       CURTEMP = CURTEMP + 1;
     }
     CURLEN = CURLEN + 1;
   }
   FX_Free(LENCOUNT);
   FX_Free(FIRSTCODE);
 }