libgraphics: Correct error handling in MCGPathIterate()

peter-b · peter-b · commit 996c253ca64c · 2017-02-20T14:26:31.000Z
In `MCGPathIterate()`, the `t_success` flag was not checked before
invoking the `p_callback`.  If the default case in the `switch` block
was hit, this resulted in calling `p_callback` with uninitialised
values from the stack.

This patch addresses the issue in two ways:

- All values in the stack are initialised at acquisition
- The function now returns immediately on encountering a problem; the
  `t_success` flag has been removed entirely
diff --git a/libgraphics/src/path.cpp b/libgraphics/src/path.cpp
@@ -1163,22 +1163,20 @@ bool MCGPathIterate(MCGPathRef self, MCGPathIterateCallback p_callback, void *p_
 {
 	if (!MCGPathIsValid(self))
 		return false;
-	
-	bool t_success;
-	t_success = true;
-	
+
 	MCGPoint t_points[3];
-	uint32_t t_point_count;
-	MCGPathCommand t_command;
+	uint32_t t_point_count = 0;
+	MCGPathCommand t_command = kMCGPathCommandEnd;
 	
 	SkPath::Iter t_iter(*self->path, false);
 	SkPath::Verb t_verb;
 	SkPoint t_sk_points[4];
 	
 	// IM-2015-03-20: [[ Bug 15035 ]] The first point returned by SkPath::Iter::next() is
 	//  always the last moveTo point; the points for the current verb start at index 1.
-	while (t_success && (t_verb = t_iter.next(t_sk_points)) != SkPath::kDone_Verb)
+	while ((t_verb = t_iter.next(t_sk_points)) != SkPath::kDone_Verb)
 	{
+
 		switch(t_verb)
 		{
 			case SkPath::kMove_Verb:
@@ -1219,15 +1217,11 @@ bool MCGPathIterate(MCGPathRef self, MCGPathIterateCallback p_callback, void *p_
 				
 			default:
 				// Unknown path instruction
-				t_success = false;
-				break;
+				return false;
 		}
-		
-		t_success = p_callback(p_context, t_command, t_points, t_point_count);
+		if (!p_callback(p_context, t_command, t_points, t_point_count))
+			return false;
 	}
 	
-	if (t_success)
-		t_success = p_callback(p_context, kMCGPathCommandEnd, nil, 0);
-	
-	return t_success;
+	return p_callback(p_context, kMCGPathCommandEnd, nil, 0);
 }
