Prevent crash when accessing child count, but child list is NULL.

d16r · Facebook Github Bot · commit ced779b25966 · 2016-10-31T12:52:52.000-07:00
Summary: Previously, we would preallocate Node's with a child list of 4. We recently removed that logic (see diff below), and as a result, if you tried to access a Node's list of children before it had been allocated, you would crash. I added a simple check to protect from crashes, the operation of the check is O(1) so we shouldn't see a perf hit.

Reviewed By: emilsjolander

Differential Revision: D4104093

fbshipit-source-id: cd7b09818759aa76415b97e241f1a6746a2bc50c
diff --git a/CSSLayout/CSSNodeList.c b/CSSLayout/CSSNodeList.c
@@ -92,5 +92,9 @@ CSSNodeRef CSSNodeListDelete(const CSSNodeListRef list, const CSSNodeRef node) {
 }
 
 CSSNodeRef CSSNodeListGet(const CSSNodeListRef list, const uint32_t index) {
-  return list->items[index];
+  if (CSSNodeListCount(list) > 0) {
+    return list->items[index];
+  }
+
+  return NULL;
 }
diff --git a/tests/CSSLayoutDefaultValuesTest.cpp b/tests/CSSLayoutDefaultValuesTest.cpp
@@ -13,6 +13,9 @@
 TEST(CSSLayoutTest, assert_default_values) {
   const CSSNodeRef root = CSSNodeNew();
 
+  ASSERT_EQ(0, CSSNodeChildCount(root));
+  ASSERT_EQ(NULL, CSSNodeGetChild(root, 1));
+
   ASSERT_EQ(CSSDirectionInherit, CSSNodeStyleGetDirection(root));
   ASSERT_EQ(CSSFlexDirectionColumn, CSSNodeStyleGetFlexDirection(root));
   ASSERT_EQ(CSSJustifyFlexStart, CSSNodeStyleGetJustifyContent(root));

Original file line number	Diff line number	Diff line change
`@@ -92,5 +92,9 @@ CSSNodeRef CSSNodeListDelete(const CSSNodeListRef list, const CSSNodeRef node) {`
`92`	`92`	`}`
`93`	`93`
`94`	`94`	`CSSNodeRef CSSNodeListGet(const CSSNodeListRef list, const uint32_t index) {`
`95`		`- return list->items[index];`
	`95`	`+ if (CSSNodeListCount(list) > 0) {`
	`96`	`+ return list->items[index];`
	`97`	`+ }`
	`98`	`+`
	`99`	`+ return NULL;`
`96`	`100`	`}`