Fix uninitialized coords in _DrawCoonPatchMeshes

core/src/fpdfapi/fpdf_render/fpdf_render_pattern.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 "../../../include/fpdfapi/fpdf_render.h"
 #include "../../../include/fpdfapi/fpdf_pageobj.h"
 #include "../../../include/fxge/fx_ge.h"
 #include "../fpdf_page/pageint.h"
@@ skipping 670 matching lines @@
     patch.alpha = alpha;
     patch.pDevice = &device;
     patch.fill_mode = fill_mode;
     patch.path.SetPointCount(13);
     FX_PATHPOINT* pPoints = patch.path.GetPoints();
     pPoints[0].m_Flag = FXPT_MOVETO;
     for (int i = 1; i < 13; i ++) {
         pPoints[i].m_Flag = FXPT_BEZIERTO;
     }
     CFX_FloatPoint coords[16];
+    for (int i = 0; i < 16; i ++)

[palmer, 2014/07/10 23:11:01]

This is fine.

But a more elegant solution is to declare that the default constructor of
|CFX_FloatPoint| sets its member fields to 0 (or whatever). The default
constructor will be called for each element of the array.

[jun_fang, 2014/07/15 01:13:41]

CFX_FloatPoint is created by CFX_PSVTTemplate<FX_FLOAT>. This template supports
couple types. 

[palmer, 2014/07/15 01:49:17]

Yes. I am saying, you could give that template a default constructor. Currently
it does not have one.

[jun_fang, 2014/07/15 03:56:17]

:-). I know. I tried to add a default constructor. However there is a minor
issue that we may use 0 to initialize a float/double variable because we don't
know which type will be created. We can only use one type to initialize. 

[Ryan Sleevi, 2014/07/15 23:59:00]

Just happened to see this CL go by. The nice thing with templates is you can use
the "typename T" and T() (default initializer) to zero-initialize numeric types.

In C++ language, this is known as the "Default Constructible" requirement
(20.1.5 of C++03), which defines the requirement that T() must be a well-formed
expression.

Among the STL, you have many types require default-constructible; among them,
vectors and maps (which also zero-initialize). This behaviour (for primitives)
is defined in Section 8.5 of C++03. In particular, the language of 8.5 subclause
5 and 7.

That is, the initializer for all scalar types (which are a subset of the object
types clause) have a default initializer (), which is a value-initializer
(subclause 7). The default value-initializer for a scalar type is to
zero-initialize (subclause 5).

It's an extremely portable way to zero-initialize all types T, where T may be a
scalar type.

You're guaranteed that the CFX_FloatPoint constructor (really,
CFX_PSVVTTemplate<FX_FLOAT> constructor) will be run for every member in the
array (subclause 5 dictates that array types have every member
value-initialized), ergo if your constructor value-initialized the coords using
T() [Or, in the case of CFX_PSVTemplate, "baseType()"], this would be entirely
unnecessary.

To express this in an initializer list, you could do

CFX_PSVTemplate::CFX_PSVTemplate() : x(), y() {}

which would appropriately value-initialize (ergo zero-initialize)

+    {
+        coords[i].Set(0.0f, 0.0f);
+    }
+
     int point_count = bTensor ? 16 : 12;
     while (!stream.m_BitStream.IsEOF()) {
         FX_DWORD flag = stream.GetFlag();

[palmer, 2014/07/10 23:11:01]

Because this value is untrustworthy (comes from the PDF itself, right?) and is
unchecked, ...

[jun_fang, 2014/07/15 01:13:41]

The value from GetFlag() is limited to 0 to 3. The definition of GetFlag is
shown as below: 

FX_DWORD CPDF_MeshStream::GetFlag()
{
    return m_BitStream.GetBits(m_nFlagBits) & 0x03;
}

[palmer, 2014/07/15 01:49:17]

Acknowledged.

         int iStartPoint = 0, iStartColor = 0, i;
         if (flag) {
             iStartPoint = 4;
             iStartColor = 2;
             CFX_FloatPoint tempCoords[4];
             for (int i = 0; i < 4; i ++) {
                 tempCoords[i] = coords[(flag * 3 + i) % 12];
             }
             FXSYS_memcpy32(coords, tempCoords, sizeof(CFX_FloatPoint) * 4);
             Coon_Color tempColors[2];
             tempColors[0] = patch.patch_colors[flag];

[palmer, 2014/07/10 23:11:01]

... this could be an out-of-bounds read, I think.

[jun_fang, 2014/07/15 01:13:41]

flag should be 0 to 3 as explained before. So it's safe.

[palmer, 2014/07/15 01:49:17]

Acknowledged.

             tempColors[1] = patch.patch_colors[(flag + 1) % 4];
             FXSYS_memcpy32(patch.patch_colors, tempColors, sizeof(Coon_Color) * 2);
         }
         for (i = iStartPoint; i < point_count; i ++) {
             stream.GetCoords(coords[i].x, coords[i].y);
             pObject2Bitmap->Transform(coords[i].x, coords[i].y);
         }
         for (i = iStartColor; i < 4; i ++) {
             FX_FLOAT r=0.0f, g=0.0f, b=0.0f;
             stream.GetColor(r, g, b);
@@ skipping 369 matching lines @@
             bStroke = FALSE;
             bPattern = TRUE;
         }
     }
 #ifdef _FPDFAPI_MINI_
     if (bPattern && m_DitherBits) {
         DitherObjectArea(pPathObj, pObj2Device);
     }
 #endif
 }