From 1bf12c858fe92d20f30da22d5d9ea4b7068c5f46958dcda9cf3f8c3c9e212c8a Mon Sep 17 00:00:00 2001 From: ADAM David Alan Martin Date: Mon, 8 Sep 2025 13:24:56 -0400 Subject: [PATCH] String alloc shouldn't convert to `std::string`. As I work through making code use more C++ RAII and such, most of the work is handling strings, especially temporaries. As member variables which manage string memory get turned into `std::string`, some use cases might wind up leaking memory. (One was found in this change.) By using a non-convertible-to-string result, such accidents should be avoided. --- dlib/dlib.cc | 20 ++++++++++---------- dlib/dlib.hh | 15 +++++++++++---- dpi/cookies.cc | 2 +- dpi/datauri.cc | 2 +- dw/fltkui.cc | 6 +++--- dw/hyphenator.cc | 2 +- lout/object.cc | 2 +- src/form.cc | 2 +- 8 files changed, 29 insertions(+), 22 deletions(-) diff --git a/dlib/dlib.cc b/dlib/dlib.cc index a8b0809..6e6a6b4 100644 --- a/dlib/dlib.cc +++ b/dlib/dlib.cc @@ -74,32 +74,32 @@ void dFree (void *mem) *- strings (char *) ---------------------------------------------------------- */ -char *dStrdup(const char *s) +CharPtrNoStringConversion dStrdup(const char *s) { if (s) { int len = strlen(s)+1; char *ns = dNew(char, len); memcpy(ns, s, len); - return ns; + return { ns }; } - return NULL; + return {}; } -char *dStrndup(const char *s, size_t sz) +CharPtrNoStringConversion dStrndup(const char *s, size_t sz) { if (s) { char *ns = dNew(char, sz+1); memcpy(ns, s, sz); ns[sz] = 0; - return ns; + return { ns }; } - return NULL; + return {}; } /** * Concatenate a NULL-terminated list of strings */ -char *dStrconcat(const char *s1, ...) +CharPtrNoStringConversion dStrconcat(const char *s1, ...) { va_list args; char *s, *ns = NULL; @@ -113,7 +113,7 @@ char *dStrconcat(const char *s1, ...) ns = dstr->str; dStr_free(dstr, 0); } - return ns; + return { ns }; } /** @@ -146,11 +146,11 @@ void dStrshred(char *s) /** * Return a new string of length 'len' filled with 'c' characters */ -char *dStrnfill(size_t len, char c) +CharPtrNoStringConversion dStrnfill(size_t len, char c) { char *ret = dNew(char, len+1); for (ret[len] = 0; len > 0; ret[--len] = c); - return ret; + return { ret }; } /** diff --git a/dlib/dlib.hh b/dlib/dlib.hh index c437553..91341d3 100644 --- a/dlib/dlib.hh +++ b/dlib/dlib.hh @@ -11,6 +11,13 @@ #include "d_size.h" +struct CharPtrNoStringConversion +{ + char *ptr= nullptr; + + operator char *() const noexcept { return ptr; } +}; + #ifdef __cplusplus extern "C" { #endif /* __cplusplus */ @@ -84,11 +91,11 @@ void dFree (void *mem); /* *- C strings ----------------------------------------------------------------- */ -char *dStrdup(const char *s); -char *dStrndup(const char *s, size_t sz); -char *dStrconcat(const char *s1, ...); +CharPtrNoStringConversion dStrdup(const char *s); +CharPtrNoStringConversion dStrndup(const char *s, size_t sz); +CharPtrNoStringConversion dStrconcat(const char *s1, ...); char *dStrstrip(char *s); -char *dStrnfill(size_t len, char c); +CharPtrNoStringConversion dStrnfill(size_t len, char c); char *dStrsep(char **orig, const char *delim); void dStrshred(char *s); char *dStriAsciiStr(const char *haystack, const char *needle); diff --git a/dpi/cookies.cc b/dpi/cookies.cc index 23e9f29..ed71bf4 100644 --- a/dpi/cookies.cc +++ b/dpi/cookies.cc @@ -1329,7 +1329,7 @@ static std::string Cookies_get(char *url_host, char *url_path, int i; if (disabled) - return dStrdup(""); + return ""; matching_cookies = dList_new(8); diff --git a/dpi/datauri.cc b/dpi/datauri.cc index f3f7db6..f2ec810 100644 --- a/dpi/datauri.cc +++ b/dpi/datauri.cc @@ -274,7 +274,7 @@ static unsigned char *datauri_get_data(char *url, size_t *p_sz) data = (unsigned char *)a_Url_decode_hex_str(p, p_sz); } } else { - data = (unsigned char *)dStrdup(""); + data = (unsigned char *)dStrdup("").ptr; *p_sz = 0; } diff --git a/dw/fltkui.cc b/dw/fltkui.cc index 0676754..33cbc61 100644 --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -866,9 +866,9 @@ FltkEntryResource::FltkEntryResource (FltkPlatform *platform, int size, { this->size = size; this->password = password; - this->label = label ? dStrdup(label) : NULL; + this->label = dStrdup(label); this->label_w = 0; - this->placeholder = placeholder ? dStrdup(placeholder) : NULL; + this->placeholder = dStrdup(placeholder); initText = NULL; editable = false; @@ -1052,7 +1052,7 @@ FltkMultiLineTextResource::FltkMultiLineTextResource (FltkPlatform *platform, MSG_WARN("numRows = %d is set to 1.\n", numRows); numRows = 1; } - this->placeholder = placeholder ? dStrdup(placeholder) : NULL; + this->placeholder = dStrdup(placeholder); init (platform); } diff --git a/dw/hyphenator.cc b/dw/hyphenator.cc index 779b74c..9ee4b61 100644 --- a/dw/hyphenator.cc +++ b/dw/hyphenator.cc @@ -371,7 +371,7 @@ TrieBuilder::~TrieBuilder () void TrieBuilder::insert (const char *key, const char *value) { dataList->increase (); - dataList->getLastRef ()->key = (unsigned char *) dStrdup(key); + dataList->getLastRef ()->key = (unsigned char *) dStrdup(key).ptr; dataList->getLastRef ()->value = dataZone->strdup (value); } diff --git a/lout/object.cc b/lout/object.cc index b3809d4..cdbfd4a 100644 --- a/lout/object.cc +++ b/lout/object.cc @@ -290,7 +290,7 @@ void ConstString::intoStringBuffer(misc::StringBuffer *sb) const // String // ------------ -String::String (const char *str): ConstString (str ? dStrdup(str) : NULL) +String::String (const char *str): ConstString (dStrdup(str)) { } diff --git a/src/form.cc b/src/form.cc index 1a6ec9a..23f68df 100644 --- a/src/form.cc +++ b/src/form.cc @@ -861,7 +861,7 @@ void Html_tag_open_option(DilloHtml *html, const char *tag, int tagsize) bool enabled = (a_Html_get_attr(html, tag, tagsize, "disabled") == NULL); auto option = - std::make_unique< DilloHtmlOption >(value ? dStrdup( value.value().c_str() ) : nullptr, label ? dStrdup( label.value().c_str() ) : nullptr, selected, enabled ); + std::make_unique< DilloHtmlOption >(value ? dStrdup( value.value().c_str() ).ptr : nullptr, label ? dStrdup( label.value().c_str() ).ptr : nullptr, selected, enabled ); input->select->addOpt( std::move( option ) ); }