From 90e1de790e6a16fe7410c43d1566c218139cd0c2 Mon Sep 17 00:00:00 2001 From: Dennis Park Date: Mon, 3 Aug 2026 18:07:56 +0900 Subject: [PATCH] fix: treat appending an empty string as a no-op, not an error (0.6.5) CMUTIL_StringAddNString folded two unrelated conditions into one branch: a NULL pointer, which is a genuine programming error, and a size of zero, which is not. Appending nothing should quietly do nothing and report the unchanged size. Instead every such call returned -1 and logged an ERROR with a stack trace. The consequence surfaced through Replace. Removing a substring is spelled Replace(needle, ""), and the empty replacement made AddString fail, which sent Replace to its failure path: it destroyed the result and returned NULL. Stripping the dash out of "UTF-8" - a textbook use of Replace - was exactly the combination that broke, and callers that dereferenced the result crashed. XmlSetName has the same shape (Clear then AddString) and logged an ERROR with a stack trace on every empty name, drowning real errors in noise. AddNString now rejects NULL and returns the current size for a zero-size append. AddString, Replace and XmlSetName all follow from that one change. InsertNString carried the same defect and is fixed the same way, with one difference: the index is validated before the size is considered, because an out of bound index stays an error even when there is nothing to insert. InsertString and InsertAnother were missing the NULL guard their Add counterparts already had, and crashed on a NULL argument instead of reporting it. Replace also dropped the return value of the AddString that appends the tail after the last match, while checking it inside the loop. Both are checked now. The header documented only "New size of this string object" for these methods, with nothing about error returns or empty input. The contract is now written down: an empty append or insert is a no-op returning the unchanged size, NULL returns -1, an out of bound index returns -1, and an empty alter passed to Replace removes every occurrence of the needle. --- VERSION | 2 +- src/libcmutils.h | 47 +++++++++++++++++---- src/strings.c | 102 ++++++++++++++++++++++++++------------------- test/string_test.c | 100 ++++++++++++++++++++++++++++++++++++++++++++ test/xml_test.c | 15 +++++++ vcpkg.json | 2 +- 6 files changed, 216 insertions(+), 52 deletions(-) diff --git a/VERSION b/VERSION index d2b13eb..ef5e445 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.6.4 +0.6.5 diff --git a/src/libcmutils.h b/src/libcmutils.h index 196d24a..65c57bc 100644 --- a/src/libcmutils.h +++ b/src/libcmutils.h @@ -1426,9 +1426,13 @@ struct CMUTIL_String { * * Append the given string to the end of this string object. * + * Appending an empty string is not an error, it just does nothing. + * * @param string This string object. * @param tobeadded C-style null terminated string to be appended. - * @return New size of this string object. + * @return New size of this string object, which is the unchanged current + * size when @a tobeadded is an empty string. + * -1 if @a tobeadded is NULL. */ ssize_t (*AddString)( CMUTIL_String *string, const char *tobeadded); @@ -1439,10 +1443,14 @@ struct CMUTIL_String { * Append the given string to the end of this string object. * Given string is not needed to be null terminated. * + * Appending zero bytes is not an error, it just does nothing. + * * @param string This string object. * @param tobeadded C style string to be appended. * @param size Number of bytes to be appended from the given string. - * @return New size of this string object. + * @return New size of this string object, which is the unchanged current + * size when @a size is zero. + * -1 if @a tobeadded is NULL. */ ssize_t (*AddNString)( CMUTIL_String *string, const char *tobeadded, size_t size); @@ -1490,9 +1498,14 @@ struct CMUTIL_String { * * Append the given string object to the end of this string object. * + * Appending an empty string object is not an error, + * it just does nothing. + * * @param string This string object. * @param tobeadded Another string object to be appended. - * @return New size of this string object. + * @return New size of this string object, which is the unchanged current + * size when @a tobeadded is an empty string object. + * -1 if @a tobeadded is NULL. */ ssize_t (*AddAnother)( CMUTIL_String *string, const CMUTIL_String *tobeadded); @@ -1502,10 +1515,16 @@ struct CMUTIL_String { * * Insert the given string to this string object at the index. * + * Inserting an empty string is not an error, it just does nothing, + * but the index is still validated. + * * @param string This string object. * @param tobeadded C style null terminated string to be inserted. * @param at Index where the given string will be inserted. - * @return New size of this string object. + * @return New size of this string object, which is the unchanged current + * size when @a tobeadded is an empty string. + * -1 if @a tobeadded is NULL or @a at is greater than the current + * size of this string object. */ ssize_t (*InsertString)( CMUTIL_String *string, const char *tobeadded, uint32_t at); @@ -1517,11 +1536,17 @@ struct CMUTIL_String { * at the given index. * Given string is not needed to be null terminated. * + * Inserting zero bytes is not an error, it just does nothing, + * but the index is still validated. + * * @param string This string object. * @param tobeadded C style string to be inserted. * @param at Index where the given string will be inserted. * @param size Number of bytes to be inserted from the given string. - * @return New size of this string object. + * @return New size of this string object, which is the unchanged current + * size when @a size is zero. + * -1 if @a tobeadded is NULL or @a at is greater than the current + * size of this string object. */ ssize_t (*InsertNString)( CMUTIL_String *string, @@ -1561,10 +1586,16 @@ struct CMUTIL_String { * * Insert the given string object to this string object at the index. * + * Inserting an empty string object is not an error, it just does nothing, + * but the index is still validated. + * * @param string This string object. * @param idx Index where another string object will be inserted. * @param tobeadded Another string object to be inserted. - * @return New size of this string object. + * @return New size of this string object, which is the unchanged current + * size when @a tobeadded is an empty string object. + * -1 if @a tobeadded is NULL or @a idx is greater than the current + * size of this string object. */ ssize_t (*InsertAnother)( CMUTIL_String *string, uint32_t idx, CMUTIL_String *tobeadded); @@ -1635,9 +1666,11 @@ struct CMUTIL_String { * * @param string This string object. * @param needle Substring to be replaced. - * @param alter Replacement string. + * @param alter Replacement string. An empty string is allowed and + * removes every occurrence of @a needle. * @return New string object which is the result of replacement. * Must be destroyed after use. + * NULL if @a needle or @a alter is NULL, or on internal failure. */ CMUTIL_String *(*Replace)( const CMUTIL_String *string, diff --git a/src/strings.c b/src/strings.c index bc91406..efd2804 100644 --- a/src/strings.c +++ b/src/strings.c @@ -251,27 +251,29 @@ CMUTIL_STATIC ssize_t CMUTIL_StringAddString(CMUTIL_String *string, CMLogError("CMUTIL_String AddNString failed"); return ret; } - CMLogErrorS("invalid argument. %s", tobeadded == NULL ? "NULL" : ""); + CMLogErrorS("invalid argument. NULL"); return -1; } CMUTIL_STATIC ssize_t CMUTIL_StringAddNString(CMUTIL_String *string, const char *tobeadded, const size_t size) { - if (tobeadded && size > 0) { - CMUTIL_String_Internal *istr = (CMUTIL_String_Internal*)string; - if (!CMUTIL_StringCheckSize(istr, size)) { - CMLogError("CMUTIL_StringCheckSize failed"); - return -1; - } - memcpy(istr->data + istr->size, tobeadded, size); - istr->size += size; - istr->data[istr->size] = 0x0; + CMUTIL_String_Internal *istr = (CMUTIL_String_Internal*)string; + if (tobeadded == NULL) { + CMLogErrorS("invalid argument. NULL"); + return -1; + } + // appending nothing is not an error, it just does nothing. + if (size == 0) return (ssize_t)istr->size; + if (!CMUTIL_StringCheckSize(istr, size)) { + CMLogError("CMUTIL_StringCheckSize failed"); + return -1; } - CMLogErrorS("invalid argument. %s, size: %zu", - tobeadded == NULL ? "NULL" : "", size); - return -1; + memcpy(istr->data + istr->size, tobeadded, size); + istr->size += size; + istr->data[istr->size] = 0x0; + return (ssize_t)istr->size; } CMUTIL_STATIC ssize_t CMUTIL_StringAddChar(CMUTIL_String *string, char tobeadded) @@ -343,42 +345,49 @@ CMUTIL_STATIC ssize_t CMUTIL_StringAddAnother( CMLogError("CMUTIL_String AddNString failed"); return ret; } - CMLogErrorS("invalid argument. %s", tobeadded == NULL ? "NULL" : ""); + CMLogErrorS("invalid argument. NULL"); return -1; } CMUTIL_STATIC ssize_t CMUTIL_StringInsertString(CMUTIL_String *string, const char *tobeadded, const uint32_t at) { - size_t size = strlen((const char*)tobeadded); - const ssize_t ret = CMCall(string, InsertNString, tobeadded, at, size); - if (ret < 0) - CMLogError("CMUTIL_String InsertNString failed"); - return ret; + if (tobeadded) { + const size_t size = strlen(tobeadded); + const ssize_t ret = CMCall(string, InsertNString, tobeadded, at, size); + if (ret < 0) + CMLogError("CMUTIL_String InsertNString failed"); + return ret; + } + CMLogErrorS("invalid argument. NULL"); + return -1; } CMUTIL_STATIC ssize_t CMUTIL_StringInsertNString(CMUTIL_String *string, const char *tobeadded, const uint32_t at, const size_t size) { - if (tobeadded && size > 0) { - CMUTIL_String_Internal *istr = (CMUTIL_String_Internal*)string; - if ((size_t)at > istr->size) { - CMLogErrorS("index out of bound. at: %u, size: %zu", - at, istr->size); - return -1; - } - if (!CMUTIL_StringCheckSize(istr, size)) { - CMLogError("CMUTIL_StringCheckSize failed"); - return -1; - } - memmove(istr->data+at+size, istr->data+at, istr->size-(size_t)at+1); - memcpy(istr->data+at, tobeadded, size); - istr->size += size; + CMUTIL_String_Internal *istr = (CMUTIL_String_Internal*)string; + if (tobeadded == NULL) { + CMLogErrorS("invalid argument. NULL"); + return -1; + } + // an out of bound index is an error even if nothing is to be inserted. + if ((size_t)at > istr->size) { + CMLogErrorS("index out of bound. at: %u, size: %zu", + at, istr->size); + return -1; + } + // inserting nothing is not an error, it just does nothing. + if (size == 0) return (ssize_t)istr->size; + if (!CMUTIL_StringCheckSize(istr, size)) { + CMLogError("CMUTIL_StringCheckSize failed"); + return -1; } - CMLogErrorS("invalid argument. %s, size: %zu", - tobeadded == NULL ? "NULL" : "", size); - return -1; + memmove(istr->data+at+size, istr->data+at, istr->size-(size_t)at+1); + memcpy(istr->data+at, tobeadded, size); + istr->size += size; + return (ssize_t)istr->size; } CMUTIL_STATIC ssize_t CMUTIL_StringInsertPrint( @@ -428,12 +437,16 @@ CMUTIL_STATIC ssize_t CMUTIL_StringInsertVPrint( CMUTIL_STATIC ssize_t CMUTIL_StringInsertAnother( CMUTIL_String *string, uint32_t idx, CMUTIL_String *tobeadded) { - const char *str = CMCall(tobeadded, GetCString); - ssize_t len = CMCall(tobeadded, GetSize); - len = CMCall(string, InsertNString, str, idx, len); - if (len < 0) - CMLogError("CMUTIL_String InsertNString failed"); - return len; + if (tobeadded) { + const char *str = CMCall(tobeadded, GetCString); + const size_t len = CMCall(tobeadded, GetSize); + const ssize_t ret = CMCall(string, InsertNString, str, idx, len); + if (ret < 0) + CMLogError("CMUTIL_String InsertNString failed"); + return ret; + } + CMLogErrorS("invalid argument. NULL"); + return -1; } CMUTIL_STATIC void CMUTIL_StringCutTailOff( @@ -563,7 +576,10 @@ CMUTIL_STATIC CMUTIL_String *CMUTIL_StringReplace( cur += nlen; prv = cur; cur = strstr(prv, needle); } - CMCall(res, AddString, prv); + if (CMCall(res, AddString, prv) < 0) { + CMLogError("CMUTIL_String AddString failed"); + goto FAIL_POINT; + } } else { CMLogError("CMUTIL_StringCreateInternal failed"); } diff --git a/test/string_test.c b/test/string_test.c index 2475a52..eac21cf 100644 --- a/test/string_test.c +++ b/test/string_test.c @@ -107,6 +107,106 @@ int main() { CMCall(str, SelfTrim); ASSERT(strcmp(CMCall(str, GetCString), "test") == 0, "SelfTrim"); + // empty append/insert must be a no-op, not an error. + CMCall(str, Clear); + ASSERT(CMCall(str, AddString, "") == 0, "AddString empty on empty string"); + ASSERT(strcmp(CMCall(str, GetCString), "") == 0, + "AddString empty on empty string validation"); + + CMCall(str, AddString, "abc"); + ASSERT(CMCall(str, AddString, "") == 3, "AddString empty"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "AddString empty validation"); + + ASSERT(CMCall(str, AddNString, "xyz", 0) == 3, "AddNString zero size"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "AddNString zero size validation"); + + ASSERT(CMCall(str, AddString, NULL) == -1, "AddString NULL"); + ASSERT(CMCall(str, AddNString, NULL, 3) == -1, "AddNString NULL"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "AddString/AddNString NULL validation"); + + ASSERT(CMCall(str, InsertString, "", 1) == 3, "InsertString empty"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "InsertString empty validation"); + + ASSERT(CMCall(str, InsertNString, "x", 1, 0) == 3, + "InsertNString zero size"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "InsertNString zero size validation"); + + ASSERT(CMCall(str, InsertString, NULL, 0) == -1, "InsertString NULL"); + ASSERT(CMCall(str, InsertNString, NULL, 0, 1) == -1, "InsertNString NULL"); + + // an out of bound index is still an error even with nothing to insert. + ASSERT(CMCall(str, InsertNString, "x", 10, 0) == -1, + "InsertNString zero size out of bound"); + ASSERT(CMCall(str, InsertString, "", 10) == -1, + "InsertString empty out of bound"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "Insert out of bound validation"); + + // a NULL string object is an error, an empty one is a no-op. + ASSERT(CMCall(str, AddAnother, NULL) == -1, "AddAnother NULL"); + ASSERT(CMCall(str, InsertAnother, 1, NULL) == -1, "InsertAnother NULL"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "AddAnother/InsertAnother NULL validation"); + + if (another) CMCall(another, Destroy); another = NULL; + another = CMUTIL_StringCreate(); + ASSERT(CMCall(str, AddAnother, another) == 3, "AddAnother empty"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "AddAnother empty validation"); + + ASSERT(CMCall(str, InsertAnother, 1, another) == 3, "InsertAnother empty"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "InsertAnother empty validation"); + + // an out of bound index is still an error even with nothing to insert. + ASSERT(CMCall(str, InsertAnother, 10, another) == -1, + "InsertAnother empty out of bound"); + ASSERT(strcmp(CMCall(str, GetCString), "abc") == 0, + "InsertAnother empty out of bound validation"); + + CMCall(another, AddString, "XY"); + ASSERT(CMCall(str, InsertAnother, 1, another) == 5, "InsertAnother"); + ASSERT(strcmp(CMCall(str, GetCString), "aXYbc") == 0, + "InsertAnother validation"); + + ASSERT(CMCall(str, InsertAnother, 10, another) == -1, + "InsertAnother out of bound"); + ASSERT(strcmp(CMCall(str, GetCString), "aXYbc") == 0, + "InsertAnother out of bound validation"); + + // replacing with an empty string removes every occurrence of needle. + CMCall(str, Clear); + CMCall(str, AddString, "UTF-8"); + if (another) CMCall(another, Destroy); another = NULL; + another = CMCall(str, Replace, "-", ""); + ASSERT(another != NULL, "Replace with empty alter"); + ASSERT(strcmp(CMCall(another, GetCString), "UTF8") == 0, + "Replace with empty alter validation"); + ASSERT(strcmp(CMCall(str, GetCString), "UTF-8") == 0, + "Replace with empty alter keeps source"); + + CMCall(str, Clear); + CMCall(str, AddString, "--a--b--"); + if (another) CMCall(another, Destroy); another = NULL; + another = CMCall(str, Replace, "-", ""); + ASSERT(another != NULL, "Replace leading/trailing/consecutive needle"); + ASSERT(strcmp(CMCall(another, GetCString), "ab") == 0, + "Replace leading/trailing/consecutive needle validation"); + ASSERT(strcmp(CMCall(str, GetCString), "--a--b--") == 0, + "Replace leading/trailing/consecutive needle keeps source"); + + if (another) CMCall(another, Destroy); another = NULL; + another = CMCall(str, Replace, "--", "-"); + ASSERT(another != NULL, "Replace non-empty alter"); + ASSERT(strcmp(CMCall(another, GetCString), "-a-b-") == 0, + "Replace non-empty alter validation"); + ASSERT(strcmp(CMCall(str, GetCString), "--a--b--") == 0, + "Replace non-empty alter keeps source"); ////////////////////////////////////////////////////////////////////// // CMUTIL_StringArray tests diff --git a/test/xml_test.c b/test/xml_test.c index 83b0381..048303b 100644 --- a/test/xml_test.c +++ b/test/xml_test.c @@ -2,6 +2,8 @@ // Created by 박성진 on 25. 12. 23.. // +#include + #include "libcmutils.h" #include "test.h" @@ -12,6 +14,7 @@ int main() { CMUTIL_Init(CMUTIL_MEM_TYPE); CMUTIL_XmlNode *node = NULL; + CMUTIL_XmlNode *tmpnode = NULL; CMUTIL_String *str = NULL; CMUTIL_Json *json = NULL; @@ -24,6 +27,17 @@ int main() { node = CMUTIL_XmlParse(str); ASSERT(node != NULL, "CMUTIL_XmlParse"); + // setting an empty name must succeed and yield an empty name. + tmpnode = CMUTIL_XmlNodeCreate(CMXmlNodeTag, "tmp"); + ASSERT(tmpnode != NULL, "CMUTIL_XmlNodeCreate"); + ASSERT(strcmp(CMCall(tmpnode, GetName), "tmp") == 0, "XmlNode GetName"); + CMCall(tmpnode, SetName, ""); + ASSERT(strcmp(CMCall(tmpnode, GetName), "") == 0, "XmlNode SetName empty"); + CMCall(tmpnode, SetName, "renamed"); + ASSERT(strcmp(CMCall(tmpnode, GetName), "renamed") == 0, + "XmlNode SetName"); + CMCall(tmpnode, Destroy); tmpnode = NULL; + if (str) CMCall(str, Destroy); str = NULL; str = CMCall(node, ToDocument, CMFalse); @@ -39,6 +53,7 @@ int main() { END_POINT: if (json) CMUTIL_JsonDestroy(json); if (str) CMCall(str, Destroy); + if (tmpnode) CMCall(tmpnode, Destroy); if (node) CMCall(node, Destroy); if (!CMUTIL_Clear()) ir = -1; return ir; diff --git a/vcpkg.json b/vcpkg.json index ccfdb09..3b60ed6 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -1,6 +1,6 @@ { "name" : "libcmutils", - "version-string" : "0.6.4", + "version-string" : "0.6.5", "builtin-baseline" : "594ad8871e1e8e45f8e626c015fd611163430207", "dependencies" : [ { "name" : "libiconv",