commit 1c577cd3ee331944b9061ee0eec147b211ee563c
author mtklein <mtklein@chromium.org> Wed Jul 09 09:54:10 2014 -0700
committer Commit bot <commit-bot@chromium.org> Wed Jul 09 09:54:11 2014 -0700
tree ca4e2ea83c7b2343844c4e60d5d7c03353178d30
parent 2e09d18f78f5772d2d9506ff8024ca8542654358

Always initialize SkDashPathEffect fields.

SkDashPathEffect is flattened into a hash key as part of the old-and-busted
SkPicture paint deduping code.  If all its fields aren't intialized, this hash
will be based on uninitialized data.  This means the hash won't be
deterministic, and worse, Valgrind and MSAN will make us feel bad.

An alternative to this is to have SkDashPath::CalcDashParameters always
guarantee it writes something to all its output parameters, even when the dash
intervals make no sense.  I like it being dumb and its users defensive, but
could go either way.

BUG=391001
R=reed@google.com, mtklein@google.com

Author: mtklein@chromium.org

Review URL: https://codereview.chromium.org/379813004

src/effects/SkDashPathEffect.cpp

diff --git a/src/effects/SkDashPathEffect.cpp b/src/effects/SkDashPathEffect.cpp
index 2838b1f..d91e7be 100644
--- a/src/effects/SkDashPathEffect.cpp
+++ b/src/effects/SkDashPathEffect.cpp
@@ -11,8 +11,11 @@
 #include "SkReadBuffer.h"
 #include "SkWriteBuffer.h"
 
-SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count,
-                                   SkScalar phase) {
+SkDashPathEffect::SkDashPathEffect(const SkScalar intervals[], int count, SkScalar phase)
+        : fPhase(0)
+        , fInitialDashLength(0)
+        , fInitialDashIndex(0)
+        , fIntervalLength(0) {
     SkASSERT(intervals);
     SkASSERT(count > 1 && SkAlign2(count) == count);
 
@@ -24,8 +27,8 @@
     }
 
     // set the internal data members
-    SkDashPath::CalcDashParameters(phase, fIntervals, fCount, &fInitialDashLength,
-                                   &fInitialDashIndex, &fIntervalLength, &fPhase);
+    SkDashPath::CalcDashParameters(phase, fIntervals, fCount,
+            &fInitialDashLength, &fInitialDashIndex, &fIntervalLength, &fPhase);
 }
 
 SkDashPathEffect::~SkDashPathEffect() {
@@ -249,7 +252,12 @@
     return SkNEW_ARGS(SkDashPathEffect, (buffer));
 }
 
-SkDashPathEffect::SkDashPathEffect(SkReadBuffer& buffer) : INHERITED(buffer) {
+SkDashPathEffect::SkDashPathEffect(SkReadBuffer& buffer)
+        : INHERITED(buffer)
+        , fPhase(0)
+        , fInitialDashLength(0)
+        , fInitialDashIndex(0)
+        , fIntervalLength(0) {
     bool useOldPic = buffer.isVersionLT(SkReadBuffer::kDashWritesPhaseIntervals_Version);
     if (useOldPic) {
         fInitialDashIndex = buffer.readInt();
@@ -280,7 +288,7 @@
     } else {
         // set the internal data members, fPhase should have been between 0 and intervalLength
         // when written to buffer so no need to adjust it
-        SkDashPath::CalcDashParameters(fPhase, fIntervals, fCount, &fInitialDashLength,
-                                       &fInitialDashIndex, &fIntervalLength);
+        SkDashPath::CalcDashParameters(fPhase, fIntervals, fCount,
+                &fInitialDashLength, &fInitialDashIndex, &fIntervalLength);
     }
 }