From ef81d4b0c757ad2e4942a7edcc16c28817faf152 Mon Sep 17 00:00:00 2001 From: Kazuki Sakamoto Date: Tue, 15 Nov 2016 20:20:09 -0800 Subject: [PATCH] Introduce CSSLayoutSetMemoryFuncs Summary: - Add CSSLayoutSetMemoryFuncs function which allows application to replace malloc, calloc, realloc and free with application's functions, like zalloc and zfree of zlib. - Fixed memory leaks in tests. - Close #247 For example, to use dlmalloc with USE_DL_PREFIX CSSLayoutSetMemoryFuncs(&dlmalloc, &dlcalloc, &dlrealloc, &dlfree); Reviewed By: emilsjolander Differential Revision: D4178386 fbshipit-source-id: a79dbdaf82a512f42cc43f99dbc49faba296903b --- CSSLayout/CSSLayout.c | 32 ++++++++++++- CSSLayout/CSSLayout.h | 10 ++++ CSSLayout/CSSNodeList.c | 14 ++++-- tests/CSSLayoutDirtyMarkingTest.cpp | 5 +- tests/CSSLayoutMeasureTest.cpp | 3 ++ tests/CSSLayoutMemoryFuncTest.cpp | 71 +++++++++++++++++++++++++++++ 6 files changed, 127 insertions(+), 8 deletions(-) create mode 100644 tests/CSSLayoutMemoryFuncTest.cpp diff --git a/CSSLayout/CSSLayout.c b/CSSLayout/CSSLayout.c index dacaceec..6dc0f7bc 100644 --- a/CSSLayout/CSSLayout.c +++ b/CSSLayout/CSSLayout.c @@ -102,6 +102,11 @@ typedef struct CSSNode { static void _CSSNodeMarkDirty(const CSSNodeRef node); +CSSMalloc gCSSMalloc = &malloc; +CSSCalloc gCSSCalloc = &calloc; +CSSRealloc gCSSRealloc = &realloc; +CSSFree gCSSFree = &free; + #ifdef ANDROID #include static int _csslayoutAndroidLog(CSSLogLevel level, const char *format, va_list args) { @@ -178,7 +183,7 @@ static inline float computedEdgeValue(const float edges[CSSEdgeCount], static int32_t gNodeInstanceCount = 0; CSSNodeRef CSSNodeNew(void) { - const CSSNodeRef node = calloc(1, sizeof(CSSNode)); + const CSSNodeRef node = gCSSCalloc(1, sizeof(CSSNode)); CSS_ASSERT(node, "Could not allocate memory for node"); gNodeInstanceCount++; @@ -199,7 +204,7 @@ void CSSNodeFree(const CSSNodeRef node) { } CSSNodeListFree(node->children); - free(node); + gCSSFree(node); gNodeInstanceCount--; } @@ -2523,3 +2528,26 @@ void CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeature feature, bool bool CSSLayoutIsExperimentalFeatureEnabled(CSSExperimentalFeature feature) { return experimentalFeatures[feature]; } + +void CSSLayoutSetMemoryFuncs(CSSMalloc cssMalloc, + CSSCalloc cssCalloc, + CSSRealloc cssRealloc, + CSSFree cssFree) { + CSS_ASSERT(gNodeInstanceCount == 0, + "Cannot set memory functions: all node must be freed first"); + CSS_ASSERT((cssMalloc == NULL && cssCalloc == NULL && cssRealloc == NULL && cssFree == NULL) || + (cssMalloc != NULL && cssCalloc != NULL && cssRealloc != NULL && cssFree != NULL), + "Cannot set memory functions: functions must be all NULL or Non-NULL"); + + if (cssMalloc == NULL || cssCalloc == NULL || cssRealloc == NULL || cssFree == NULL) { + gCSSMalloc = &malloc; + gCSSCalloc = &calloc; + gCSSRealloc = &realloc; + gCSSFree = &free; + } else { + gCSSMalloc = cssMalloc; + gCSSCalloc = cssCalloc; + gCSSRealloc = cssRealloc; + gCSSFree = cssFree; + } +} diff --git a/CSSLayout/CSSLayout.h b/CSSLayout/CSSLayout.h index 391a458f..07a8f6ec 100644 --- a/CSSLayout/CSSLayout.h +++ b/CSSLayout/CSSLayout.h @@ -47,6 +47,11 @@ typedef CSSSize (*CSSMeasureFunc)(CSSNodeRef node, typedef void (*CSSPrintFunc)(CSSNodeRef node); typedef int (*CSSLogger)(CSSLogLevel level, const char *format, va_list args); +typedef void *(*CSSMalloc)(size_t size); +typedef void *(*CSSCalloc)(size_t count, size_t size); +typedef void *(*CSSRealloc)(void *ptr, size_t size); +typedef void (*CSSFree)(void *ptr); + // CSSNode WIN_EXPORT CSSNodeRef CSSNodeNew(void); WIN_EXPORT void CSSNodeInit(const CSSNodeRef node); @@ -156,4 +161,9 @@ WIN_EXPORT void CSSLog(CSSLogLevel level, const char *message, ...); WIN_EXPORT void CSSLayoutSetExperimentalFeatureEnabled(CSSExperimentalFeature feature, bool enabled); WIN_EXPORT bool CSSLayoutIsExperimentalFeatureEnabled(CSSExperimentalFeature feature); +WIN_EXPORT void CSSLayoutSetMemoryFuncs(CSSMalloc cssMalloc, + CSSCalloc cssCalloc, + CSSRealloc cssRealloc, + CSSFree cssFree); + CSS_EXTERN_C_END diff --git a/CSSLayout/CSSNodeList.c b/CSSLayout/CSSNodeList.c index d48cf4b3..886f78cb 100644 --- a/CSSLayout/CSSNodeList.c +++ b/CSSLayout/CSSNodeList.c @@ -9,6 +9,10 @@ #include "CSSNodeList.h" +extern CSSMalloc gCSSMalloc; +extern CSSRealloc gCSSRealloc; +extern CSSFree gCSSFree; + struct CSSNodeList { uint32_t capacity; uint32_t count; @@ -16,12 +20,12 @@ struct CSSNodeList { }; CSSNodeListRef CSSNodeListNew(const uint32_t initialCapacity) { - const CSSNodeListRef list = malloc(sizeof(struct CSSNodeList)); + const CSSNodeListRef list = gCSSMalloc(sizeof(struct CSSNodeList)); CSS_ASSERT(list != NULL, "Could not allocate memory for list"); list->capacity = initialCapacity; list->count = 0; - list->items = malloc(sizeof(CSSNodeRef) * list->capacity); + list->items = gCSSMalloc(sizeof(CSSNodeRef) * list->capacity); CSS_ASSERT(list->items != NULL, "Could not allocate memory for items"); return list; @@ -29,8 +33,8 @@ CSSNodeListRef CSSNodeListNew(const uint32_t initialCapacity) { void CSSNodeListFree(const CSSNodeListRef list) { if (list) { - free(list->items); - free(list); + gCSSFree(list->items); + gCSSFree(list); } } @@ -56,7 +60,7 @@ void CSSNodeListInsert(CSSNodeListRef *listp, const CSSNodeRef node, const uint3 if (list->count == list->capacity) { list->capacity *= 2; - list->items = realloc(list->items, sizeof(CSSNodeRef) * list->capacity); + list->items = gCSSRealloc(list->items, sizeof(CSSNodeRef) * list->capacity); CSS_ASSERT(list->items != NULL, "Could not extend allocation for items"); } diff --git a/tests/CSSLayoutDirtyMarkingTest.cpp b/tests/CSSLayoutDirtyMarkingTest.cpp index 7bafe352..fed544f3 100644 --- a/tests/CSSLayoutDirtyMarkingTest.cpp +++ b/tests/CSSLayoutDirtyMarkingTest.cpp @@ -83,11 +83,14 @@ TEST(CSSLayoutTest, dirty_node_only_if_children_are_actually_removed) { CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR); - CSSNodeRemoveChild(root, CSSNodeNew()); + const CSSNodeRef child1 = CSSNodeNew(); + CSSNodeRemoveChild(root, child1); EXPECT_FALSE(CSSNodeIsDirty(root)); + CSSNodeFree(child1); CSSNodeRemoveChild(root, child0); EXPECT_TRUE(CSSNodeIsDirty(root)); + CSSNodeFree(child0); CSSNodeFreeRecursive(root); } diff --git a/tests/CSSLayoutMeasureTest.cpp b/tests/CSSLayoutMeasureTest.cpp index e31063c2..23ff7da3 100644 --- a/tests/CSSLayoutMeasureTest.cpp +++ b/tests/CSSLayoutMeasureTest.cpp @@ -43,6 +43,8 @@ TEST(CSSLayoutTest, dont_measure_single_grow_shrink_child) { CSSNodeCalculateLayout(root, CSSUndefined, CSSUndefined, CSSDirectionLTR); ASSERT_EQ(0, measureCount); + + CSSNodeFreeRecursive(root); } #if GTEST_HAS_DEATH_TEST @@ -52,6 +54,7 @@ TEST(CSSLayoutTest, cannot_add_child_to_node_with_measure_func) { const CSSNodeRef root_child0 = CSSNodeNew(); ASSERT_DEATH(CSSNodeInsertChild(root, root_child0, 0), "Cannot add child.*"); + CSSNodeFree(root_child0); CSSNodeFreeRecursive(root); } diff --git a/tests/CSSLayoutMemoryFuncTest.cpp b/tests/CSSLayoutMemoryFuncTest.cpp new file mode 100644 index 00000000..ade4b6f7 --- /dev/null +++ b/tests/CSSLayoutMemoryFuncTest.cpp @@ -0,0 +1,71 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +#include +#include + +static int testMallocCount; +static int testCallocCount; +static int testReallocCount; +static int testFreeCount; + +static void *testMalloc(size_t size) { + testMallocCount++; + return malloc(size); +} + +static void *testCalloc(size_t count, size_t size) { + testCallocCount++; + return calloc(count, size); +} + +static void *testRealloc(void *ptr, size_t size) { + testReallocCount++; + return realloc(ptr, size); +} + +static void testFree(void *ptr) { + testFreeCount++; + free(ptr); +} + +TEST(CSSLayoutTest, memory_func_default) { + CSSLayoutSetMemoryFuncs(NULL, NULL, NULL, NULL); + const CSSNodeRef root = CSSNodeNew(); + const CSSNodeRef root_child0 = CSSNodeNew(); + CSSNodeInsertChild(root, root_child0, 0); + CSSNodeFreeRecursive(root); +} + +TEST(CSSLayoutTest, memory_func_test_funcs) { + CSSLayoutSetMemoryFuncs(&testMalloc, &testCalloc, &testRealloc, &testFree); + const CSSNodeRef root = CSSNodeNew(); + for (int i = 0; i < 10; i++) { + const CSSNodeRef child = CSSNodeNew(); + CSSNodeInsertChild(root, child, 0); + } + CSSNodeFreeRecursive(root); + ASSERT_NE(testMallocCount, 0); + ASSERT_NE(testCallocCount, 0); + ASSERT_NE(testReallocCount, 0); + ASSERT_NE(testFreeCount, 0); + CSSLayoutSetMemoryFuncs(NULL, NULL, NULL, NULL); +} + +#if GTEST_HAS_DEATH_TEST +TEST(CSSLayoutTest, memory_func_assert_zero_nodes) { + const CSSNodeRef root = CSSNodeNew(); + ASSERT_DEATH(CSSLayoutSetMemoryFuncs(&testMalloc, &testCalloc, &testRealloc, &testFree), "Cannot set memory functions: all node must be freed first"); + CSSNodeFreeRecursive(root); +} + +TEST(CSSLayoutTest, memory_func_assert_all_non_null) { + ASSERT_DEATH(CSSLayoutSetMemoryFuncs(NULL, &testCalloc, &testRealloc, &testFree), "Cannot set memory functions: functions must be all NULL or Non-NULL"); +} +#endif