From 126fc99c77998cc65094d92e955347100aa42531 Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 28 Apr 2022 14:10:53 -0400 Subject: [PATCH] Fix leaks in common/json, common/keyValue, and common/variantList. This doesn't solve the problem of the variant code making far too many copies, but it at least plugs the leaks. jsonReadVarRecurse() could leak KeyValue and VariantList. kvDup() leaked object allocations into the calling context. kvDefault() gets a more efficient return structure. kvGetList() leaked a Variant into the calling context. varLstNewStrLst() leaked object allocations into the calling context. Update varLstDup() to reflect changes made in varLstNewStrLst(). --- src/common/type/json.c | 24 ++++++++++---- src/common/type/keyValue.c | 60 +++++++++++++++++------------------ src/common/type/variantList.c | 28 +++++++++------- 3 files changed, 64 insertions(+), 48 deletions(-) diff --git a/src/common/type/json.c b/src/common/type/json.c index f07141f65..eaef80c54 100644 --- a/src/common/type/json.c +++ b/src/common/type/json.c @@ -1185,16 +1185,23 @@ jsonReadVarRecurse(JsonRead *const this) case jsonTypeArrayBegin: { - VariantList *const result = varLstNew(); + VariantList *const list = varLstNew(); jsonReadArrayBegin(this); - while (jsonReadTypeNextIgnoreComma(this) != jsonTypeArrayEnd) - varLstAdd(result, jsonReadVarRecurse(this)); + MEM_CONTEXT_BEGIN(lstMemContext((List *)list)) + { + while (jsonReadTypeNextIgnoreComma(this) != jsonTypeArrayEnd) + varLstAdd(list, jsonReadVarRecurse(this)); + } + MEM_CONTEXT_END(); jsonReadArrayEnd(this); - FUNCTION_TEST_RETURN(VARIANT, varNewVarLst(result)); + Variant *const result = varNewVarLst(list); + varLstFree(list); + + FUNCTION_TEST_RETURN(VARIANT, result); } default: @@ -1205,8 +1212,13 @@ jsonReadVarRecurse(JsonRead *const this) while (jsonReadTypeNextIgnoreComma(this) != jsonTypeObjectEnd) { - const String *const key = jsonReadKey(this); - kvPut(result, varNewStr(key), jsonReadVarRecurse(this)); + String *const key = jsonReadKey(this); + Variant *const value = jsonReadVarRecurse(this); + + kvPut(result, VARSTR(key), value); + + strFree(key); + varFree(value); } jsonReadObjectEnd(this); diff --git a/src/common/type/keyValue.c b/src/common/type/keyValue.c index bbdb9485b..6ca6e7679 100644 --- a/src/common/type/keyValue.c +++ b/src/common/type/keyValue.c @@ -57,7 +57,7 @@ kvNew(void) /**********************************************************************************************************************************/ KeyValue * -kvDup(const KeyValue *source) +kvDup(const KeyValue *const source) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(KEY_VALUE, source); @@ -65,23 +65,21 @@ kvDup(const KeyValue *source) ASSERT(source != NULL); - KeyValue *this = kvNew(); + KeyValue *const this = kvNew(); - // Duplicate all key/values - for (unsigned int listIdx = 0; listIdx < lstSize(source->list); listIdx++) + MEM_CONTEXT_OBJ_BEGIN(this) { - const KeyValuePair *sourcePair = (const KeyValuePair *)lstGet(source->list, listIdx); + // Duplicate all key/values + for (unsigned int listIdx = 0; listIdx < lstSize(source->list); listIdx++) + { + const KeyValuePair *const sourcePair = (const KeyValuePair *)lstGet(source->list, listIdx); + lstAdd(this->list, &(KeyValuePair){.key = varDup(sourcePair->key), .value = varDup(sourcePair->value)}); + } - // Copy the pair - KeyValuePair pair; - pair.key = varDup(sourcePair->key); - pair.value = varDup(sourcePair->value); - - // Add to the list - lstAdd(this->list, &pair); + // Duplicate key list + this->pub.keyList = varLstDup(kvKeyList(source)); } - - this->pub.keyList = varLstDup(kvKeyList(source)); + MEM_CONTEXT_OBJ_END(); FUNCTION_TEST_RETURN(KEY_VALUE, this); } @@ -281,7 +279,7 @@ kvGet(const KeyValue *this, const Variant *key) /**********************************************************************************************************************************/ const Variant * -kvGetDefault(const KeyValue *this, const Variant *key, const Variant *defaultValue) +kvGetDefault(const KeyValue *const this, const Variant *const key, const Variant *const defaultValue) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(KEY_VALUE, this); @@ -292,23 +290,19 @@ kvGetDefault(const KeyValue *this, const Variant *key, const Variant *defaultVal ASSERT(this != NULL); ASSERT(key != NULL); - const Variant *result = NULL; - // Find the key - unsigned int listIdx = kvGetIdx(this, key); + const unsigned int listIdx = kvGetIdx(this, key); // If key not found then return default, else return the value if (listIdx == KEY_NOT_FOUND) - result = defaultValue; - else - result = ((KeyValuePair *)lstGet(this->list, listIdx))->value; + FUNCTION_TEST_RETURN_CONST(VARIANT, defaultValue); - FUNCTION_TEST_RETURN_CONST(VARIANT, result); + FUNCTION_TEST_RETURN(VARIANT, ((KeyValuePair *)lstGet(this->list, listIdx))->value); } /**********************************************************************************************************************************/ VariantList * -kvGetList(const KeyValue *this, const Variant *key) +kvGetList(const KeyValue *const this, const Variant *const key) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(KEY_VALUE, this); @@ -318,16 +312,22 @@ kvGetList(const KeyValue *this, const Variant *key) ASSERT(this != NULL); ASSERT(key != NULL); - VariantList *result = NULL; - // Get the value - const Variant *value = kvGet(this, key); + const Variant *const value = kvGet(this, key); - // Convert the value to a list if it is not already one + // Return the list if it already is one if (value != NULL && varType(value) == varTypeVariantList) - result = varLstDup(varVarLst(value)); - else - result = varLstAdd(varLstNew(), varDup(value)); + FUNCTION_TEST_RETURN(VARIANT_LIST, varLstDup(varVarLst(value))); + + // Create a list to return + VariantList *result; + VariantList *const list = varLstNew(); + + MEM_CONTEXT_OBJ_BEGIN(list) + { + result = varLstAdd(list, varDup(value)); + } + MEM_CONTEXT_OBJ_END(); FUNCTION_TEST_RETURN(VARIANT_LIST, result); } diff --git a/src/common/type/variantList.c b/src/common/type/variantList.c index 30237999c..ff628b1bb 100644 --- a/src/common/type/variantList.c +++ b/src/common/type/variantList.c @@ -14,46 +14,50 @@ Variant List Handler /**********************************************************************************************************************************/ VariantList * -varLstNewStrLst(const StringList *stringList) +varLstNewStrLst(const StringList *const stringList) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(STRING_LIST, stringList); FUNCTION_TEST_END(); - VariantList *result = NULL; + VariantList *this = NULL; if (stringList != NULL) { - result = varLstNew(); + this = varLstNew(); - for (unsigned int listIdx = 0; listIdx < strLstSize(stringList); listIdx++) - varLstAdd(result, varNewStr(strLstGet(stringList, listIdx))); + MEM_CONTEXT_OBJ_BEGIN(this) + { + for (unsigned int listIdx = 0; listIdx < strLstSize(stringList); listIdx++) + varLstAdd(this, varNewStr(strLstGet(stringList, listIdx))); + } + MEM_CONTEXT_OBJ_END(); } - FUNCTION_TEST_RETURN(VARIANT_LIST, result); + FUNCTION_TEST_RETURN(VARIANT_LIST, this); } /**********************************************************************************************************************************/ VariantList * -varLstDup(const VariantList *source) +varLstDup(const VariantList *const source) { FUNCTION_TEST_BEGIN(); FUNCTION_TEST_PARAM(VARIANT_LIST, source); FUNCTION_TEST_END(); - VariantList *result = NULL; + VariantList *this = NULL; if (source != NULL) { - result = varLstNew(); + this = varLstNew(); - MEM_CONTEXT_OBJ_BEGIN(result) + MEM_CONTEXT_OBJ_BEGIN(this) { for (unsigned int listIdx = 0; listIdx < varLstSize(source); listIdx++) - varLstAdd(result, varDup(varLstGet(source, listIdx))); + varLstAdd(this, varDup(varLstGet(source, listIdx))); } MEM_CONTEXT_OBJ_END(); } - FUNCTION_TEST_RETURN(VARIANT_LIST, result); + FUNCTION_TEST_RETURN(VARIANT_LIST, this); }