Get rid of gotos in CPDF_SyntaxParser and FlateUncompress().

core/src/fxcodec/codec/fx_codec_flate.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 "../../../../third_party/base/nonstd_unique_ptr.h"
 #include "../../../../third_party/zlib_v128/zlib.h"
 #include "../../../include/fxcodec/fx_codec.h"
 #include "../../../include/fxcodec/fx_codec_flate.h"
@@ skipping 57 matching lines @@
 }
 int FPDFAPI_FlateGetAvailOut(void* context) {
   return ((z_stream*)context)->avail_out;
 }
 void FPDFAPI_FlateEnd(void* context) {
   inflateEnd((z_stream*)context);
   ((z_stream*)context)->zfree(0, context);
 }
 }  // extern "C"
 
+namespace {
+
 class CLZWDecoder {
  public:
   int Decode(uint8_t* output,
              FX_DWORD& outlen,
              const uint8_t* input,
              FX_DWORD& size,
              FX_BOOL bEarlyChange);
 
  private:
   void AddCode(FX_DWORD prefix_code, uint8_t append_char);
@@ skipping 127 matching lines @@
       } else {
         AddCode(old_code, last_char);
       }
       old_code = code;
     }
   }
   dest_size = m_OutPos;
   src_size = (m_InPos + 7) / 8;
   return 0;
 }
-static uint8_t PaethPredictor(int a, int b, int c) {
+
+uint8_t PaethPredictor(int a, int b, int c) {
   int p = a + b - c;
   int pa = FXSYS_abs(p - a);
   int pb = FXSYS_abs(p - b);
   int pc = FXSYS_abs(p - c);
   if (pa <= pb && pa <= pc) {
     return (uint8_t)a;
   }
   if (pb <= pc) {
     return (uint8_t)b;
   }
   return (uint8_t)c;
 }
-static FX_BOOL PNG_PredictorEncode(uint8_t*& data_buf,
-                                   FX_DWORD& data_size,
-                                   int predictor,
-                                   int Colors,
-                                   int BitsPerComponent,
-                                   int Columns) {
+
+FX_BOOL PNG_PredictorEncode(uint8_t*& data_buf,
+                            FX_DWORD& data_size,
+                            int predictor,
+                            int Colors,
+                            int BitsPerComponent,
+                            int Columns) {
   const int BytesPerPixel = (Colors * BitsPerComponent + 7) / 8;
   const int row_size = (Colors * BitsPerComponent * Columns + 7) / 8;
   if (row_size <= 0)
     return FALSE;
   const int row_count = (data_size + row_size - 1) / row_size;
   const int last_row_size = data_size % row_size;
   uint8_t* dest_buf = FX_Alloc2D(uint8_t, row_size + 1, row_count);
   int byte_cnt = 0;
   uint8_t* pSrcData = data_buf;
   uint8_t* pDestData = dest_buf;
@@ skipping 63 matching lines @@
     }
     pDestData += (row_size + 1);
     pSrcData += row_size;
   }
   FX_Free(data_buf);
   data_buf = dest_buf;
   data_size = (row_size + 1) * row_count -
               (last_row_size > 0 ? (row_size - last_row_size) : 0);
   return TRUE;
 }
-static void PNG_PredictLine(uint8_t* pDestData,
-                            const uint8_t* pSrcData,
-                            const uint8_t* pLastLine,
-                            int bpc,
-                            int nColors,
-                            int nPixels) {
+
+void PNG_PredictLine(uint8_t* pDestData,
+                     const uint8_t* pSrcData,
+                     const uint8_t* pLastLine,
+                     int bpc,
+                     int nColors,
+                     int nPixels) {
   int row_size = (nPixels * bpc * nColors + 7) / 8;
   int BytesPerPixel = (bpc * nColors + 7) / 8;
   uint8_t tag = pSrcData[0];
   if (tag == 0) {
     FXSYS_memmove(pDestData, pSrcData + 1, row_size);
     return;
   }
   for (int byte = 0; byte < row_size; byte++) {
     uint8_t raw_byte = pSrcData[byte + 1];
     switch (tag) {
@@ skipping 40 matching lines @@
         }
         pDestData[byte] = raw_byte + PaethPredictor(left, up, upper_left);
         break;
       }
       default:
         pDestData[byte] = raw_byte;
         break;
     }
   }
 }
-static FX_BOOL PNG_Predictor(uint8_t*& data_buf,
-                             FX_DWORD& data_size,
-                             int Colors,
-                             int BitsPerComponent,
-                             int Columns) {
+
+FX_BOOL PNG_Predictor(uint8_t*& data_buf,
+                      FX_DWORD& data_size,
+                      int Colors,
+                      int BitsPerComponent,
+                      int Columns) {
   const int BytesPerPixel = (Colors * BitsPerComponent + 7) / 8;
   const int row_size = (Colors * BitsPerComponent * Columns + 7) / 8;
   if (row_size <= 0)
     return FALSE;
   const int row_count = (data_size + row_size) / (row_size + 1);
   const int last_row_size = data_size % (row_size + 1);
   uint8_t* dest_buf = FX_Alloc2D(uint8_t, row_size, row_count);
   int byte_cnt = 0;
   uint8_t* pSrcData = data_buf;
   uint8_t* pDestData = dest_buf;
@@ skipping 66 matching lines @@
     }
     pSrcData += row_size + 1;
     pDestData += row_size;
   }
   FX_Free(data_buf);
   data_buf = dest_buf;
   data_size = row_size * row_count -
               (last_row_size > 0 ? (row_size + 1 - last_row_size) : 0);
   return TRUE;
 }
-static void TIFF_PredictorEncodeLine(uint8_t* dest_buf,
-                                     int row_size,
-                                     int BitsPerComponent,
-                                     int Colors,
-                                     int Columns) {
+
+void TIFF_PredictorEncodeLine(uint8_t* dest_buf,
+                              int row_size,
+                              int BitsPerComponent,
+                              int Colors,
+                              int Columns) {
   int BytesPerPixel = BitsPerComponent * Colors / 8;
   if (BitsPerComponent < 8) {
     uint8_t mask = 0x01;
     if (BitsPerComponent == 2) {
       mask = 0x03;
     } else if (BitsPerComponent == 4) {
       mask = 0x0F;
     }
     int row_bits = Colors * BitsPerComponent * Columns;
     for (int i = row_bits - BitsPerComponent; i >= BitsPerComponent;
@@ skipping 20 matching lines @@
     for (int i = row_size - BytesPerPixel; i >= BytesPerPixel;
          i -= BytesPerPixel) {
       FX_WORD pixel = (dest_buf[i] << 8) | dest_buf[i + 1];
       pixel -=
           (dest_buf[i - BytesPerPixel] << 8) | dest_buf[i - BytesPerPixel + 1];
       dest_buf[i] = pixel >> 8;
       dest_buf[i + 1] = (uint8_t)pixel;
     }
   }
 }
-static FX_BOOL TIFF_PredictorEncode(uint8_t*& data_buf,
-                                    FX_DWORD& data_size,
-                                    int Colors,
-                                    int BitsPerComponent,
-                                    int Columns) {
+
+FX_BOOL TIFF_PredictorEncode(uint8_t*& data_buf,
+                             FX_DWORD& data_size,
+                             int Colors,
+                             int BitsPerComponent,
+                             int Columns) {
   int row_size = (Colors * BitsPerComponent * Columns + 7) / 8;
   if (row_size == 0)
     return FALSE;
   const int row_count = (data_size + row_size - 1) / row_size;
   const int last_row_size = data_size % row_size;
   for (int row = 0; row < row_count; row++) {
     uint8_t* scan_line = data_buf + row * row_size;
     if ((row + 1) * row_size > (int)data_size) {
       row_size = last_row_size;
     }
     TIFF_PredictorEncodeLine(scan_line, row_size, BitsPerComponent, Colors,
                              Columns);
   }
   return TRUE;
 }
-static void TIFF_PredictLine(uint8_t* dest_buf,
-                             int row_size,
-                             int BitsPerComponent,
-                             int Colors,
-                             int Columns) {
+
+void TIFF_PredictLine(uint8_t* dest_buf,
+                      int row_size,
+                      int BitsPerComponent,
+                      int Colors,
+                      int Columns) {
   if (BitsPerComponent == 1) {
     int row_bits = FX_MIN(BitsPerComponent * Colors * Columns, row_size * 8);
     int index_pre = 0;
     int col_pre = 0;
     for (int i = 1; i < row_bits; i++) {
       int col = i % 8;
       int index = i / 8;
       if (((dest_buf[index] >> (7 - col)) & 1) ^
           ((dest_buf[index_pre] >> (7 - col_pre)) & 1)) {
         dest_buf[index] |= 1 << (7 - col);
@@ skipping 13 matching lines @@
       pixel += (dest_buf[i] << 8) | dest_buf[i + 1];
       dest_buf[i] = pixel >> 8;
       dest_buf[i + 1] = (uint8_t)pixel;
     }
   } else {
     for (int i = BytesPerPixel; i < row_size; i++) {
       dest_buf[i] += dest_buf[i - BytesPerPixel];
     }
   }
 }
-static FX_BOOL TIFF_Predictor(uint8_t*& data_buf,
-                              FX_DWORD& data_size,
-                              int Colors,
-                              int BitsPerComponent,
-                              int Columns) {
+
+FX_BOOL TIFF_Predictor(uint8_t*& data_buf,
+                       FX_DWORD& data_size,
+                       int Colors,
+                       int BitsPerComponent,
+                       int Columns) {
   int row_size = (Colors * BitsPerComponent * Columns + 7) / 8;
   if (row_size == 0)
     return FALSE;
   const int row_count = (data_size + row_size - 1) / row_size;
   const int last_row_size = data_size % row_size;
   for (int row = 0; row < row_count; row++) {
     uint8_t* scan_line = data_buf + row * row_size;
     if ((row + 1) * row_size > (int)data_size) {
       row_size = last_row_size;
     }
     TIFF_PredictLine(scan_line, row_size, BitsPerComponent, Colors, Columns);
   }
   return TRUE;
 }
 
+void FlateUncompress(const uint8_t* src_buf,

[Lei Zhang, 2015/09/23 19:22:28]

A bit harder to see what I changed since I moved this function.

- With the gotos gone, I can declare variables closer to where they are used.
- FX_Realloc() can't fail and return NULL, so those checks are gone.
- Call FPDFAPI_FlateEnd(context) on failure.
- Use a unique_ptr for |guess_buf| so there's slightly better tracking of
ownership.

+                     FX_DWORD src_size,
+                     FX_DWORD orig_size,
+                     uint8_t*& dest_buf,
+                     FX_DWORD& dest_size,
+                     FX_DWORD& offset) {
+  FX_DWORD guess_size = orig_size ? orig_size : src_size * 2;
+  FX_DWORD alloc_step =
+      orig_size ? 10240 : (src_size < 10240 ? 10240 : src_size);

[Tom Sepez, 2015/09/23 21:48:41]

nit: maybe std::min(src_size, 10240)
maybe a const FX_DWORD kMinStepSize = 10240;

[Lei Zhang, 2015/09/25 09:33:49]

Done.

+  static const FX_DWORD kMaxInitialAllocSize = 10000000;
+  if (guess_size > kMaxInitialAllocSize) {
+    guess_size = kMaxInitialAllocSize;
+    alloc_step = kMaxInitialAllocSize;
+  }
+  FX_DWORD buf_size = guess_size;
+  FX_DWORD last_buf_size = buf_size;
+
+  void* context = FPDFAPI_FlateInit(my_alloc_func, my_free_func);
+  if (!context) {
+    dest_buf = nullptr;

[Tom Sepez, 2015/09/23 21:48:41]

how about we set these to null/0 initially, and then only set them to legit
values right before a successful return?  Then this can be if (!context) return;

[Lei Zhang, 2015/09/25 09:33:48]

Done.

+    dest_size = 0;
+    return;
+  }
+
+  nonstd::unique_ptr<uint8_t, FxFreeDeleter> guess_buf(
+      FX_Alloc(uint8_t, guess_size + 1));
+  guess_buf.get()[guess_size] = '\0';

[Tom Sepez, 2015/09/23 21:48:41]

The real std::unique_ptr has operator[]. do we want to do the same before
landing this?  Do we want to new[] rather than mallocating?

[Lei Zhang, 2015/09/25 09:33:48]

There's no new equivalent to FX_Realloc(), right? I would use a std::vector if
we didn't have to send the |guess_buf| owned pointer out via |dest_buf|.

[Tom Sepez, 2015/09/25 16:25:34]

ah.

+
+  FPDFAPI_FlateInput(context, src_buf, src_size);
+
+  if (src_size < 10240) {
+    // This is the old implementation.
+    uint8_t* cur_buf = guess_buf.get();
+    while (1) {
+      int32_t ret = FPDFAPI_FlateOutput(context, cur_buf, buf_size);
+      if (ret != Z_OK)
+        break;
+      int32_t avail_buf_size = FPDFAPI_FlateGetAvailOut(context);
+      if (avail_buf_size != 0)
+        break;
+
+      // |avail_buf_size| == 0 case.

[Tom Sepez, 2015/09/23 21:48:41]

nit: not sure this comment adds value.

[Lei Zhang, 2015/09/25 09:33:48]

Done.

+      FX_DWORD old_size = guess_size;
+      guess_size += alloc_step;
+      if (guess_size < old_size || guess_size + 1 < guess_size) {
+        FPDFAPI_FlateEnd(context);
+        dest_buf = nullptr;
+        dest_size = 0;
+        return;
+      }
+
+      {
+        uint8_t* new_buf =
+            FX_Realloc(uint8_t, guess_buf.release(), guess_size + 1);
+        guess_buf.reset(new_buf);

[Tom Sepez, 2015/09/23 21:48:41]

nit: how about guess_buf.reset(FX_Realloc(...)); and lose the { }

[Lei Zhang, 2015/09/25 09:33:48]

No, we get in trouble doing guess_buf.reset() and guess_buf.release() at the
same time.

[Tom Sepez, 2015/09/25 16:25:34]

Acknowledged.

+      }
+      guess_buf.get()[guess_size] = '\0';
+      cur_buf = guess_buf.get() + old_size;
+      buf_size = guess_size - old_size;
+    }
+    dest_size = FPDFAPI_FlateGetTotalOut(context);
+    offset = FPDFAPI_FlateGetTotalIn(context);
+    if (guess_size / 2 > dest_size) {
+      {
+        uint8_t* new_buf =

[Tom Sepez, 2015/09/23 21:48:41]

ditto

[Lei Zhang, 2015/09/25 09:33:48]

Acknowledged.

+            FX_Realloc(uint8_t, guess_buf.release(), dest_size + 1);
+        guess_buf.reset(new_buf);
+      }
+      guess_size = dest_size;
+      guess_buf.get()[guess_size] = '\0';
+    }
+    dest_buf = guess_buf.release();
+  } else {
+    CFX_ArrayTemplate<uint8_t*> result_tmp_bufs;
+    uint8_t* cur_buf = guess_buf.release();
+    while (1) {
+      int32_t ret = FPDFAPI_FlateOutput(context, cur_buf, buf_size);
+      int32_t avail_buf_size = FPDFAPI_FlateGetAvailOut(context);
+      if (ret != Z_OK) {
+        last_buf_size = buf_size - avail_buf_size;
+        result_tmp_bufs.Add(cur_buf);
+        break;
+      }
+      if (avail_buf_size != 0) {
+        last_buf_size = buf_size - avail_buf_size;
+        result_tmp_bufs.Add(cur_buf);
+        break;
+      }
+
+      // |avail_buf_size| == 0 case.

[Tom Sepez, 2015/09/23 21:48:41]

ditto

[Lei Zhang, 2015/09/25 09:33:48]

Done.

+      result_tmp_bufs.Add(cur_buf);
+      cur_buf = FX_Alloc(uint8_t, buf_size + 1);
+      cur_buf[buf_size] = '\0';
+    }
+    dest_size = FPDFAPI_FlateGetTotalOut(context);
+    offset = FPDFAPI_FlateGetTotalIn(context);
+    if (result_tmp_bufs.GetSize() == 1) {
+      dest_buf = result_tmp_bufs[0];
+    } else {
+      uint8_t* result_buf = FX_Alloc(uint8_t, dest_size);
+      FX_DWORD result_pos = 0;
+      for (int32_t i = 0; i < result_tmp_bufs.GetSize(); i++) {
+        uint8_t* tmp_buf = result_tmp_bufs[i];
+        FX_DWORD tmp_buf_size = buf_size;
+        if (i == result_tmp_bufs.GetSize() - 1) {
+          tmp_buf_size = last_buf_size;
+        }
+        FXSYS_memcpy(result_buf + result_pos, tmp_buf, tmp_buf_size);
+        result_pos += tmp_buf_size;
+        FX_Free(result_tmp_bufs[i]);
+      }
+      dest_buf = result_buf;
+    }
+  }
+  FPDFAPI_FlateEnd(context);
+}
+
+}  // namespace
+
 class CCodec_FlateScanlineDecoder : public CCodec_ScanlineDecoder {
  public:
   CCodec_FlateScanlineDecoder();
   ~CCodec_FlateScanlineDecoder() override;
 
   void Create(const uint8_t* src_buf,
               FX_DWORD src_size,
               int width,
               int height,
               int nComps,
@@ skipping 135 matching lines @@
       }
     }
   } else {
     FPDFAPI_FlateOutput(m_pFlate, m_pScanline, m_Pitch);
   }
   return m_pScanline;
 }
 FX_DWORD CCodec_FlateScanlineDecoder::GetSrcOffset() {
   return FPDFAPI_FlateGetTotalIn(m_pFlate);
 }
-static void FlateUncompress(const uint8_t* src_buf,
-                            FX_DWORD src_size,
-                            FX_DWORD orig_size,
-                            uint8_t*& dest_buf,
-                            FX_DWORD& dest_size,
-                            FX_DWORD& offset) {
-  const FX_BOOL useOldImpl = src_size < 10240;
-  FX_DWORD guess_size = orig_size ? orig_size : src_size * 2;
-  FX_DWORD alloc_step =
-      orig_size ? 10240 : (src_size < 10240 ? 10240 : src_size);
-  static const FX_DWORD kMaxInitialAllocSize = 10000000;
-  if (guess_size > kMaxInitialAllocSize) {
-    guess_size = kMaxInitialAllocSize;
-    alloc_step = kMaxInitialAllocSize;
-  }
-  FX_DWORD buf_size = guess_size;
-  FX_DWORD last_buf_size = buf_size;
-  void* context = nullptr;
 
-  uint8_t* guess_buf = FX_Alloc(uint8_t, guess_size + 1);
-  uint8_t* cur_buf = guess_buf;
-  guess_buf[guess_size] = '\0';
-  context = FPDFAPI_FlateInit(my_alloc_func, my_free_func);
-  if (!context)
-    goto fail;
-  FPDFAPI_FlateInput(context, src_buf, src_size);
-  if (useOldImpl) {
-    while (1) {
-      int32_t ret = FPDFAPI_FlateOutput(context, cur_buf, buf_size);
-      if (ret != Z_OK)
-        break;
-      int32_t avail_buf_size = FPDFAPI_FlateGetAvailOut(context);
-      if (avail_buf_size != 0)
-        break;
-
-      // |avail_buf_size| == 0 case.
-      FX_DWORD old_size = guess_size;
-      guess_size += alloc_step;
-      if (guess_size < old_size || guess_size + 1 < guess_size)
-        goto fail;
-      guess_buf = FX_Realloc(uint8_t, guess_buf, guess_size + 1);
-      if (!guess_buf)
-        goto fail;
-      guess_buf[guess_size] = '\0';
-      cur_buf = guess_buf + old_size;
-      buf_size = guess_size - old_size;
-    }
-    dest_size = FPDFAPI_FlateGetTotalOut(context);
-    offset = FPDFAPI_FlateGetTotalIn(context);
-    if (guess_size / 2 > dest_size) {
-      guess_buf = FX_Realloc(uint8_t, guess_buf, dest_size + 1);
-      if (!guess_buf)
-        goto fail;
-      guess_size = dest_size;
-      guess_buf[guess_size] = '\0';
-    }
-    dest_buf = guess_buf;
-  } else {
-    CFX_ArrayTemplate<uint8_t*> result_tmp_bufs;
-    while (1) {
-      int32_t ret = FPDFAPI_FlateOutput(context, cur_buf, buf_size);
-      int32_t avail_buf_size = FPDFAPI_FlateGetAvailOut(context);
-      if (ret != Z_OK) {
-        last_buf_size = buf_size - avail_buf_size;
-        result_tmp_bufs.Add(cur_buf);
-        break;
-      }
-      if (avail_buf_size != 0) {
-        last_buf_size = buf_size - avail_buf_size;
-        result_tmp_bufs.Add(cur_buf);
-        break;
-      }
-
-      // |avail_buf_size| == 0 case.
-      result_tmp_bufs.Add(cur_buf);
-      cur_buf = FX_Alloc(uint8_t, buf_size + 1);
-      cur_buf[buf_size] = '\0';
-    }
-    dest_size = FPDFAPI_FlateGetTotalOut(context);
-    offset = FPDFAPI_FlateGetTotalIn(context);
-    if (result_tmp_bufs.GetSize() == 1) {
-      dest_buf = result_tmp_bufs[0];
-    } else {
-      uint8_t* result_buf = FX_Alloc(uint8_t, dest_size);
-      FX_DWORD result_pos = 0;
-      for (int32_t i = 0; i < result_tmp_bufs.GetSize(); i++) {
-        uint8_t* tmp_buf = result_tmp_bufs[i];
-        FX_DWORD tmp_buf_size = buf_size;
-        if (i == result_tmp_bufs.GetSize() - 1) {
-          tmp_buf_size = last_buf_size;
-        }
-        FXSYS_memcpy(result_buf + result_pos, tmp_buf, tmp_buf_size);
-        result_pos += tmp_buf_size;
-        FX_Free(result_tmp_bufs[i]);
-      }
-      dest_buf = result_buf;
-    }
-  }
-  FPDFAPI_FlateEnd(context);
-  return;
-
-fail:
-  FX_Free(guess_buf);
-  dest_buf = nullptr;
-  dest_size = 0;
-  return;
-}
 ICodec_ScanlineDecoder* CCodec_FlateModule::CreateDecoder(
     const uint8_t* src_buf,
     FX_DWORD src_size,
     int width,
     int height,
     int nComps,
     int bpc,
     int predictor,
     int Colors,
     int BitsPerComponent,
@@ skipping 87 matching lines @@
                                    FX_DWORD src_size,
                                    uint8_t*& dest_buf,
                                    FX_DWORD& dest_size) {
   dest_size = src_size + src_size / 1000 + 12;
   dest_buf = FX_Alloc(uint8_t, dest_size);
   unsigned long temp_size = dest_size;
   FPDFAPI_FlateCompress(dest_buf, &temp_size, src_buf, src_size);
   dest_size = (FX_DWORD)temp_size;
   return TRUE;
 }