pgbackrest.git / summary / log / commit / refs

commit    fe0fd71be2e9754491cbbdcde79e7a7cc87f2e7b
Author:   David Steele <david@pgmasters.net>
Date:     Thu Mar 30 05:00:58 2023 +0000

    Convert the result of zNewFmt() into an object.
    
    The result is not intended to be freed directly so this makes memory tracking more accurate. Fix a few places where memory was leaking after a call to zNewFmt().
    
    Also update an assert to make it clearer.


 src/common/type/buffer.c                |  9 ++++++---
 src/common/type/stringZ.c               | 14 ++++++++++++--
 src/storage/helper.c                    |  2 +-
 test/src/module/common/typeStringTest.c | 17 +++++++++++++++++
 test/src/module/storage/posixTest.c     |  7 ++++++-
 5 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/src/common/type/buffer.c b/src/common/type/buffer.c
index 74d9d6a06..43418936a 100644
--- a/src/common/type/buffer.c
+++ b/src/common/type/buffer.c
@@ -343,7 +343,10 @@ bufUsedZero(Buffer *this)
 FN_EXTERN void
 bufToLog(const Buffer *const this, StringStatic *const debugLog)
 {
-    strStcFmt(
-        debugLog, "{used: %zu, size: %zu%s", bufUsed(this), bufSize(this),
-        bufSizeLimit(this) ? zNewFmt(", sizeAlloc: %zu}", bufSizeAlloc(this)) : "}");
+    strStcFmt(debugLog, "{used: %zu, size: %zu", bufUsed(this), bufSize(this));
+
+    if (bufSizeLimit(this))
+        strStcFmt(debugLog, ", sizeAlloc: %zu", bufSizeAlloc(this));
+
+    strStcCatChr(debugLog, '}');
 }
diff --git a/src/common/type/stringZ.c b/src/common/type/stringZ.c
index 250ef2e37..fc6a7f3ac 100644
--- a/src/common/type/stringZ.c
+++ b/src/common/type/stringZ.c
@@ -8,6 +8,7 @@ Zero-Terminated String Handler
 
 #include "common/debug.h"
 #include "common/memContext.h"
+#include "common/type/object.h"
 #include "common/type/stringZ.h"
 
 /**********************************************************************************************************************************/
@@ -20,13 +21,22 @@ zNewFmt(const char *const format, ...)
 
     ASSERT(format != NULL);
 
-    // Determine how long the allocated string needs to be and create object
+    // Determine how long the allocated string needs to be
     va_list argumentList;
     va_start(argumentList, format);
     const size_t size = (size_t)vsnprintf(NULL, 0, format, argumentList) + 1;
-    char *const result = memNew(size);
     va_end(argumentList);
 
+    // Allocate the string as extra or a separate allocation based on how large it is
+    char *result;
+
+    OBJ_NEW_BASE_EXTRA_BEGIN(
+        char *, size > MEM_CONTEXT_ALLOC_EXTRA_MAX ? 0 : (uint16_t)size, .allocQty = size > MEM_CONTEXT_ALLOC_EXTRA_MAX ? 1 : 0)
+    {
+        result = size > MEM_CONTEXT_ALLOC_EXTRA_MAX ? memNew(size) : OBJ_NEW_ALLOC();
+    }
+    OBJ_NEW_END();
+
     // Format string
     va_start(argumentList, format);
     vsnprintf(result, size, format, argumentList);
diff --git a/src/storage/helper.c b/src/storage/helper.c
index 9ea52d37b..6b47518b5 100644
--- a/src/storage/helper.c
+++ b/src/storage/helper.c
@@ -340,7 +340,7 @@ storageRepoPathExpression(const String *const expression, const String *const pa
     else
         THROW_FMT(AssertError, "invalid expression '%s'", strZ(expression));
 
-    ASSERT(result != 0);
+    ASSERT(result != NULL);
 
     FUNCTION_TEST_RETURN(STRING, result);
 }
diff --git a/test/src/module/common/typeStringTest.c b/test/src/module/common/typeStringTest.c
index b600da3fd..1a85dee16 100644
--- a/test/src/module/common/typeStringTest.c
+++ b/test/src/module/common/typeStringTest.c
@@ -705,7 +705,24 @@ testRun(void)
     // *****************************************************************************************************************************
     if (testBegin("z*()"))
     {
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("small string");
+
         TEST_RESULT_Z(zNewFmt("id=%d", 777), "id=777", "format");
+
+        // -------------------------------------------------------------------------------------------------------------------------
+        TEST_TITLE("string large enough to need separate allocation");
+
+        char *format = memNew(MEM_CONTEXT_ALLOC_EXTRA_MAX + 1);
+        memset(format, 'A', MEM_CONTEXT_ALLOC_EXTRA_MAX);
+        format[MEM_CONTEXT_ALLOC_EXTRA_MAX] = '\0';
+        format[0] = '%';
+        format[1] = 's';
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wformat-nonliteral"
+        TEST_RESULT_Z(zNewFmt(format, "%s"), format, "compare");
+#pragma GCC diagnostic pop
     }
 
     // *****************************************************************************************************************************
diff --git a/test/src/module/storage/posixTest.c b/test/src/module/storage/posixTest.c
index b57566eea..6c885bd2b 100644
--- a/test/src/module/storage/posixTest.c
+++ b/test/src/module/storage/posixTest.c
@@ -19,7 +19,12 @@ storageTestPathExpression(const String *expression, const String *path)
     String *result = NULL;
 
     if (strcmp(strZ(expression), "<TEST>") == 0)
-        result = strNewFmt("test%s", path == NULL ? "" : zNewFmt("/%s", strZ(path)));
+    {
+        result = strCatZ(strNew(), "test");
+
+        if (path != NULL)
+            strCatFmt(result, "/%s", strZ(path));
+    }
     else if (strcmp(strZ(expression), "<NULL>") != 0)
         THROW_FMT(AssertError, "invalid expression '%s'", strZ(expression));
 


[parent: 295b53e8453b]