[PATCH v2 0/2] MR910: msvcrt: Implement dynamic string container

[Stefan Rentsch (＠restet)]

Many code sections still use fixed-size arrays on stack - or even worse - `strcat` in combination with strings of unknown length. This MR/PR provides general purpose Wine-internal functions for safer string handling. The functions are currently header-only to not pollute msvcrt exports.

The API was written for an upcoming shell32/shlexec patch (WIP) to properly fix
possible stack corruptions due to out-of-bounds writes for unexpected long paths or URLs.
This is a dynamic container inspired by C++ std::string / std::vector with
wide/narrow conversion and formatted text. Growth factor = 2 for simplicity.

Differences to `STRING`/`UNICODE_STRING` (`RtlCreateUnicodeString` and similar):

* No USHORT length limitation
 * Actual dynamic resizable container
 * String formatting and conversion support
 * Short and consistent API naming

New test added to msvcrt: `winestring`

--
  v2: Various improvements and fixes

https://gitlab.winehq.org/wine/wine/-/merge_requests/910

[PATCH v2 1/2] msvcrt: Implement dynamic string container

[Stefan Rentsch]

From: Stefan Rentsch

The API was written for an upcoming shell32/shlexec patch to properly fix
possible stack corruptions due to out-of-bounds writes.
This is a dynamic container inspired by C++ std::string / std::vector with
wide/narrow conversion and formatted text. Growth factor = 2 for simplicity.
---
 dlls/msvcrt/tests/Makefile.in  |   3 +-
 dlls/msvcrt/tests/winestring.c | 195 +++++++++++++++++++++
 include/Makefile.in            |   1 +
 include/wine/string.h          | 298 +++++++++++++++++++++++++++++++++
 4 files changed, 496 insertions(+), 1 deletion(-)
 create mode 100644 dlls/msvcrt/tests/winestring.c
 create mode 100644 include/wine/string.h

diff --git a/dlls/msvcrt/tests/Makefile.in b/dlls/msvcrt/tests/Makefile.in
index 1db6da2a0ad..572bb9b9b11 100644
--- a/dlls/msvcrt/tests/Makefile.in
+++ b/dlls/msvcrt/tests/Makefile.in
@@ -14,4 +14,5 @@ C_SRCS = \
    scanf.c \
    signal.c \
    string.c \
-	time.c
+	time.c \
+	winestring.c
diff --git a/dlls/msvcrt/tests/winestring.c b/dlls/msvcrt/tests/winestring.c
new file mode 100644
index 00000000000..b296818408e
--- /dev/null
+++ b/dlls/msvcrt/tests/winestring.c
@@ -0,0 +1,195 @@
+/*
+ * Copyright (C) the Wine project
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#include "wine/string.h"
+#include "wine/test.h"
+
+static const WCHAR *dummy_text_w = L"foo BAR /BAZ/";
+static const char  *dummy_text_a =  "foo BAR /BAZ/";
+
+static void test_init_append(void)
+{
+    size_t i;
+    {
+        /* ANSI string: grow and shrink */
+        WINEASTR str;
+        ok(WineAStrInit(&str, MAX_PATH - 1) == S_OK, "@init\n");
+
+        /* string concat */
+        ok(WineAStrAppendA(&str, "Hello world") == S_OK, "@append\n");
+        WineAStrAppendA(&str, "Hello world");
+        WineAStrAppendA(&str, "Hello world");
+        ok(str.length == 11 * 3, "len=%ld\n", str.length);
+        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+        ok(str.capacity >= str.length, "cap=%ld\n", str.capacity);
+
+        /* resize without shrink */
+        WineAStrResize(&str, str.length + 10);
+        ok(str.length == 11 * 3, "len=%ld\n", str.length);
+        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+
+        /* resize with shrink */
+        for (i = 3; i < 100; ++i)
+            WineAStrAppendA(&str, "Hello world");
+        ok(str.length == 11 * 100, "len=%ld\n", str.length);
+        WineAStrResize(&str, MAX_PATH - 1);
+        ok(str.length == MAX_PATH - 1, "len=%ld\n", str.length);
+        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+
+        WineAStrFree(&str);
+    }
+    {
+        /* Wide string: grow and shrink */
+        WINEWSTR str;
+        ok(WineWStrInit(&str, MAX_PATH - 1) == S_OK, "@init\n");
+
+        /* string concat */
+        ok(WineWStrAppendW(&str, L"Hello world") == S_OK, "@append\n");
+        WineWStrAppendW(&str, L"Hello world");
+        WineWStrAppendW(&str, L"Hello world");
+        ok(str.length == 11 * 3, "len=%ld\n", str.length);
+        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+        ok(str.capacity >= str.length, "cap=%ld\n", str.capacity);
+
+        /* resize without shrink */
+        WineWStrResize(&str, str.length + 10);
+        ok(str.length == 11 * 3, "len=%ld\n", str.length);
+        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+
+        /* resize with shrink */
+        for (i = 3; i < 100; ++i)
+            WineWStrAppendW(&str, L"Hello world");
+        ok(str.length == 11 * 100, "len=%ld\n", str.length);
+        WineWStrResize(&str, MAX_PATH - 1);
+        ok(str.length == MAX_PATH - 1, "len=%ld\n", str.length);
+        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+
+        WineWStrFree(&str);
+    }
+}
+
+static void test_replace_heap(void)
+{
+    const size_t alloc_count = 100;
+    {
+        /* ANSI string */
+        char *new_heap;
+        WINEASTR str;
+        WineAStrInit(&str, alloc_count);
+        WineAStrAppendA(&str, "data to replace");
+
+        new_heap = heap_alloc(alloc_count * sizeof(char));
+        strcpy(new_heap, dummy_text_a);
+        WineAStrReplaceHeap(&str, &new_heap);
+        ok(new_heap == NULL, "p=%p\n", new_heap);
+        ok(str.length == strlen(dummy_text_a), "len=%ld\n", str.length);
+        ok(strcmp(str.data, dummy_text_a) == 0, "@strcmp\n");
+
+        new_heap = NULL;
+        WineAStrReplaceHeap(&str, &new_heap);
+        ok(strlen(str.data) == 0, "len=%ld\n", str.length);
+
+        WineAStrFree(&str);
+    }
+    {
+        /* Unicode string */
+        WCHAR *new_heap;
+        WINEWSTR str;
+        WineWStrInit(&str, alloc_count);
+        WineWStrAppendW(&str, L"data to replace");
+
+        new_heap = heap_alloc(alloc_count * sizeof(WCHAR));
+        lstrcpyW(new_heap, dummy_text_w);
+        WineWStrReplaceHeap(&str, &new_heap);
+        ok(new_heap == NULL, "p=%p\n", new_heap);
+        ok(str.length == lstrlenW(dummy_text_w), "len=%ld\n", str.length);
+        ok(lstrcmpW(str.data, dummy_text_w) == 0, "@lstrcmpW\n");
+
+        new_heap = NULL;
+        WineWStrReplaceHeap(&str, &new_heap);
+        ok(lstrlenW(str.data) == 0, "len=%ld\n", str.length);
+
+        WineWStrFree(&str);
+    }
+}
+
+static void test_append_convert(void)
+{
+    {
+        /* ANSI string: append with width conversion */
+        WINEASTR str;
+        WineAStrInit(&str, 0);
+
+        WineAStrAppendW(&str, CP_ACP, dummy_text_w);
+        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+        ok(strcmp(str.data, dummy_text_a) == 0, "@strcmp\n");
+
+        /* repeat. double length. */
+        WineAStrAppendW(&str, CP_ACP, dummy_text_w);
+        ok(str.length == strlen(dummy_text_a) * 2, "len=%ld\n", str.length);
+
+        WineAStrFree(&str);
+    }
+    {
+        /* Wide string: append with width conversion */
+        WINEWSTR str;
+        WineWStrInit(&str, 0);
+
+        WineWStrAppendA(&str, CP_ACP, dummy_text_a);
+        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+        ok(lstrcmpW(str.data, dummy_text_w) == 0, "@lstrcmpW\n");
+
+        /* repeat. double length. */
+        WineWStrAppendA(&str, CP_ACP, dummy_text_a);
+        ok(str.length == lstrlenW(dummy_text_w) * 2, "len=%ld\n", str.length);
+
+        WineWStrFree(&str);
+    }
+}
+
+static void test_append_formatted(void)
+{
+    {
+        /* ANSI string: append with formatting */
+        WINEASTR str;
+        WineAStrInit(&str, 0);
+
+        ok(WineAStrAppendFmt(&str, "TEST %d %c+", 321, 'X') == S_OK, "@appendFmt\n");
+        ok(strcmp(str.data, "TEST 321 X+") == 0, "@strcmp, len=%ld\n", str.length);
+
+        WineAStrFree(&str);
+    }
+    {
+        /* Wide string: append with formatting */
+        WINEWSTR str;
+        WineWStrInit(&str, 0);
+
+        ok(WineWStrAppendFmt(&str, L"TEST %d %c+", 321, L'X') == S_OK, "@appendFmt\n");
+        ok(lstrcmpW(str.data, L"TEST 321 X+") == 0, "@lstrcmpW, len=%ld\n", str.length);
+
+        WineWStrFree(&str);
+    }
+}
+
+START_TEST(winestring)
+{
+    test_init_append();
+    test_replace_heap();
+    test_append_convert();
+    test_append_formatted();
+}
diff --git a/include/Makefile.in b/include/Makefile.in
index 1c04f9a298b..f495b50fabd 100644
--- a/include/Makefile.in
+++ b/include/Makefile.in
@@ -846,6 +846,7 @@ SOURCES = \
    wine/schrpc.idl \
    wine/server.h \
    wine/server_protocol.h \
+	wine/string.h \
    wine/strmbase.h \
    wine/svcctl.idl \
    wine/test.h \
diff --git a/include/wine/string.h b/include/wine/string.h
new file mode 100644
index 00000000000..15a76fcd2fa
--- /dev/null
+++ b/include/wine/string.h
@@ -0,0 +1,298 @@
+/*
+ * Copyright (C) the Wine project
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#ifndef __WINE_WINE_STRING_H
+#define __WINE_WINE_STRING_H
+
+#include <assert.h>
+#include <stdio.h> /* va_list */
+#include <windef.h> /* HRESULT status codes */
+#include <wine/heap.h>
+#include <winnls.h> /* MultiByteToWideChar */
+
+#if !defined(__WINE_USE_MSVCRT) || defined(__MINGW32__) || defined(__GNUC__)
+# define __WINE_PRINTF_ATTR(fmt,args)  __attribute__((format (printf,fmt,args)))
+/* there is no wprintf version */
+#else
+# define __WINE_PRINTF_ATTR(fmt,args)
+#endif
+
+
+/** Dynamic container for ANSI strings */
+typedef struct _WINEASTR {
+    char *data;      /**< C-string. The address may change due to realloc. */
+    size_t length;   /**< index of '\0' terminator */
+    size_t capacity; /**< allocated indices without terminator */
+} WINEASTR;
+
+/** One-time dynamic container initialization
+ * @param capacity Amount of characters to preallocate
+ */
+static HRESULT WineAStrInit(WINEASTR *str, size_t capacity)
+{
+    memset(str, 0, sizeof(*str));
+    if (!(str->data = heap_alloc((capacity + 1) * sizeof(char))))
+        return E_OUTOFMEMORY;
+
+    str->data[0] = 0;
+    str->capacity = capacity;
+    return S_OK;
+}
+
+static void WineAStrFree(WINEASTR *str)
+{
+    heap_free(str->data);
+    memset(str, 0, sizeof(*str));
+}
+
+/** Frees the existing container and takes the ownership of the provided existing heap data.
+ *  If *src_data is NULL, the container will be reinitialized with zero capacity.
+ * @param src_data Source address to takeover. The value is set to NULL on success.
+ */
+static void WineAStrReplaceHeap(WINEASTR *str, char **src_data)
+{
+    WineAStrFree(str);
+    if (*src_data) {
+        str->data = *src_data;
+        str->length = str->capacity = strlen(str->data);
+        *src_data = NULL;
+    } else {
+        WineAStrInit(str, 0);
+    }
+}
+
+/** Expand (realloc) or trim the container data
+ * @param capacity Minimal amount of characters to hold.
+ */
+static HRESULT WineAStrResize(WINEASTR *str, size_t capacity)
+{
+    if (capacity > str->capacity) {
+        char *new_ptr = heap_realloc(str->data, (capacity + 1) * sizeof(char));
+        if (!new_ptr)
+            return E_OUTOFMEMORY;
+
+        str->data = new_ptr;
+        str->capacity = capacity;
+    } else if (capacity < str->length) {
+        /* Cut off string only */
+        str->length = capacity;
+        str->data[capacity] = 0;
+    }
+    return S_OK;
+}
+
+static HRESULT WineAStrAppendA(WINEASTR *str, const char *data)
+{
+    HRESULT hr;
+    size_t len;
+
+    if (!data[0])
+        return S_OK;
+
+    len = strlen(data);
+    hr = WineAStrResize(str, (str->length + len) * 2);
+    if (hr != S_OK)
+        return hr;
+
+    memcpy(&str->data[str->length], data, (len + 1) * sizeof(char));
+    str->length += len;
+    return S_OK;
+}
+
+/** Convert and append PWSTR (WCHAR*) */
+static HRESULT WineAStrAppendW(WINEASTR *str, UINT codepage, const WCHAR *data)
+{
+    HRESULT hr;
+    size_t len;
+
+    if (!data[0])
+        return S_OK;
+
+    len = WideCharToMultiByte(codepage, 0, data, -1, NULL, 0, NULL, NULL);
+    hr = WineAStrResize(str, (str->length + len) * 2);
+    if (hr != S_OK)
+        return hr;
+
+    len = WideCharToMultiByte(codepage, 0, data, -1, &str->data[str->length], str->capacity + 1, NULL, NULL);
+    if (len <= 0)
+        return HRESULT_FROM_WIN32(GetLastError());
+
+    str->length += len - 1;
+    return S_OK;
+}
+
+/** String format and append to the container */
+static HRESULT WINAPIV __WINE_PRINTF_ATTR(2,3) WineAStrAppendFmt(WINEASTR *str, const char *fmt, ...)
+{
+    HRESULT hr;
+    int len;
+    va_list valist;
+
+    va_start(valist, fmt);
+    len = vsprintf(NULL, fmt, valist);
+    if (len < 0) {
+        hr = E_UNEXPECTED;
+        goto end;
+    }
+
+    hr = WineAStrResize(str, (str->length + len) * 2);
+    if (hr != S_OK)
+        goto end;
+
+    vsprintf(&str->data[str->length], fmt, valist);
+    str->length += len;
+
+end:
+    va_end(valist);
+    return hr;
+}
+
+/* ====================================== */
+
+/** Dynamic container for Unicode strings */
+typedef struct _WINEWSTR {
+    WCHAR *data;     /**< C-string. The address may change due to realloc. */
+    size_t length;   /**< index of '\0' terminator */
+    size_t capacity; /**< allocated indices without terminator */
+} WINEWSTR;
+
+/** One-time dynamic container initialization
+ * @param capacity Amount of characters to preallocate
+ */
+static HRESULT WineWStrInit(WINEWSTR *str, size_t capacity)
+{
+    memset(str, 0, sizeof(*str));
+    if (!(str->data = heap_alloc((capacity + 1) * sizeof(WCHAR))))
+        return E_OUTOFMEMORY;
+
+    str->data[0] = 0;
+    str->capacity = capacity;
+    return S_OK;
+}
+
+static void WineWStrFree(WINEWSTR *str)
+{
+    heap_free(str->data);
+    memset(str, 0, sizeof(*str));
+}
+
+/** Frees the existing container and takes the ownership of the provided existing heap data.
+ *  If *src_data is NULL, the container will be reinitialized with zero capacity.
+ * @param src_data Pointer to source address to takeover. The value is set to NULL on success.
+ */
+static void WineWStrReplaceHeap(WINEWSTR *str, WCHAR **src_data)
+{
+    WineWStrFree(str);
+    if (*src_data) {
+        str->data = *src_data;
+        str->length = str->capacity = lstrlenW(str->data);
+        *src_data = NULL;
+    } else {
+        WineWStrInit(str, 0);
+    }
+}
+
+/** Expand (realloc) or trim the container data
+ * @param capacity Minimal amount of characters to hold.
+ */
+static HRESULT WineWStrResize(WINEWSTR *str, size_t capacity)
+{
+    if (capacity > str->capacity) {
+        WCHAR *new_ptr = heap_realloc(str->data, (capacity + 1) * sizeof(WCHAR));
+        if (!new_ptr)
+            return E_OUTOFMEMORY;
+
+        str->data = new_ptr;
+        str->capacity = capacity;
+    } else if (capacity < str->length) {
+        /* Cut off string only */
+        str->length = capacity;
+        str->data[capacity] = 0;
+    }
+    return S_OK;
+}
+
+static HRESULT WineWStrAppendW(WINEWSTR *str, const WCHAR *data)
+{
+    HRESULT hr;
+    size_t len;
+
+    if (!data[0])
+        return S_OK;
+
+    len = lstrlenW(data);
+    hr = WineWStrResize(str, (str->length + len) * 2);
+    if (hr != S_OK)
+        return hr;
+
+    memcpy(&str->data[str->length], data, (len + 1) * sizeof(WCHAR));
+    str->length += len;
+    return S_OK;
+}
+
+/** Convert and append PSTR (char*) */
+static HRESULT WineWStrAppendA(WINEWSTR *str, UINT codepage, const char *data)
+{
+    HRESULT hr;
+    size_t len;
+
+    if (!data[0])
+        return S_OK;
+
+    len = MultiByteToWideChar(codepage, 0, data, -1, NULL, 0);
+    hr = WineWStrResize(str, (str->length + len) * 2);
+    if (hr != S_OK)
+        return hr;
+
+    len = MultiByteToWideChar(codepage, 0, data, -1, &str->data[str->length], str->capacity + 1);
+    if (len <= 0)
+        return HRESULT_FROM_WIN32(GetLastError());
+
+    str->length += len - 1;
+    return S_OK;
+}
+
+/** String format and append to the container */
+static HRESULT WINAPIV WineWStrAppendFmt(WINEWSTR *str, const WCHAR *fmt, ...)
+{
+    HRESULT hr;
+    int len;
+    va_list valist;
+
+    va_start(valist, fmt);
+    len = vswprintf(NULL, -1, fmt, valist);
+    if (len < 0) {
+        hr = E_UNEXPECTED;
+        goto end;
+    }
+
+    hr = WineWStrResize(str, (str->length + len) * 2);
+    if (hr != S_OK)
+        goto end;
+
+    vswprintf(&str->data[str->length], -1, fmt, valist);
+    str->length += len;
+
+end:
+    va_end(valist);
+    return hr;
+}
+
+#undef __WINE_PRINTF_ATTR
+
+#endif /* __WINE_WINE_STRING_H */

[PATCH v2 1/2] msvcrt: Implement dynamic string container

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=124150

Your paranoid android.

=== w10pro64_zh_CN (64 bit report) ===

msvcrt:
file.c:1743: Test failed: handle = 000000000000004C
file.c:1747: Test failed: info->handle = 000000000000004C
file.c:1750: Test failed: stdin->_file = 0
file.c:1755: Test failed: errno = 22
file.c:1765: Test failed: errno = 22
file.c:1785: Test failed: errno = 22
file.c:1789: Test failed: fclose(stdin) returned 0
file.c:1790: Test failed: errno = -559038737

=== w7u_2qxl (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w7u_adm (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w7u_el (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w8 (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w8adm (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w864 (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064v1507 (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064v1809 (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064 (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064_tsign (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w10pro64 (32 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w864 (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064v1507 (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064v1809 (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064 (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064_2qxl (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064_adm (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w1064_tsign (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w10pro64 (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w10pro64_en_AE_u8 (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w10pro64_ar (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w10pro64_ja (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

=== w10pro64_zh_CN (64 bit report) ===

msvcrt:
winestring.c:172: Test failed: @appendFmt
winestring.c:173: Test failed: @strcmp, len=0
winestring.c:182: Test failed: @appendFmt
winestring.c:183: Test failed: @lstrcmpW, len=0

[PATCH v2 2/2] Various improvements and fixes

[Stefan Rentsch]

From: Stefan Rentsch

* Fixes issues reported by buildbot
* Use of _vscprintf instead of vsprintf(NULL, ...)
* Fixes unnecessary reallocations caused by incorrect capacity checks
* String length parameter -> more use-cases
---
 dlls/msvcrt/tests/winestring.c | 117 ++++++++++++++++++++++-----------
 include/wine/string.h          |  79 +++++++++++++---------
 2 files changed, 126 insertions(+), 70 deletions(-)

diff --git a/dlls/msvcrt/tests/winestring.c b/dlls/msvcrt/tests/winestring.c
index b296818408e..6da73ca6a3e 100644
--- a/dlls/msvcrt/tests/winestring.c
+++ b/dlls/msvcrt/tests/winestring.c
@@ -19,8 +19,10 @@
 #include "wine/string.h"
 #include "wine/test.h"

-static const WCHAR *dummy_text_w = L"foo BAR /BAZ/";
-static const char  *dummy_text_a =  "foo BAR /BAZ/";
+static const WCHAR *dummy_text_w      = L"foo BAR /BAZ/";
+static const WCHAR *dummy_text_w_part = L"foo BAR /";
+static const char  *dummy_text_a      =  "foo BAR /BAZ/";
+static const char  *dummy_text_a_part =  "foo BAR /";

static void test_init_append(void)
 {
@@ -31,25 +33,25 @@ static void test_init_append(void)
         ok(WineAStrInit(&str, MAX_PATH - 1) == S_OK, "@init\n");

/* string concat */
-        ok(WineAStrAppendA(&str, "Hello world") == S_OK, "@append\n");
-        WineAStrAppendA(&str, "Hello world");
-        WineAStrAppendA(&str, "Hello world");
-        ok(str.length == 11 * 3, "len=%ld\n", str.length);
-        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+        ok(WineAStrAppend(&str, "Hello world") == S_OK, "@append\n");
+        WineAStrAppend(&str, "Hello world");
+        WineAStrAppend(&str, "Hello world");
+        ok(str.length == 11 * 3, "len=%zu\n", str.length);
+        ok(strlen(str.data) == str.length, "len=%zu\n", str.length);
         ok(str.capacity >= str.length, "cap=%ld\n", str.capacity);

/* resize without shrink */
         WineAStrResize(&str, str.length + 10);
-        ok(str.length == 11 * 3, "len=%ld\n", str.length);
-        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+        ok(str.length == 11 * 3, "len=%zu\n", str.length);
+        ok(strlen(str.data) == str.length, "len=%zu\n", str.length);

/* resize with shrink */
         for (i = 3; i < 100; ++i)
-            WineAStrAppendA(&str, "Hello world");
-        ok(str.length == 11 * 100, "len=%ld\n", str.length);
+            WineAStrAppend(&str, "Hello world");
+        ok(str.length == 11 * 100, "len=%zu\n", str.length);
         WineAStrResize(&str, MAX_PATH - 1);
-        ok(str.length == MAX_PATH - 1, "len=%ld\n", str.length);
-        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+        ok(str.length == MAX_PATH - 1, "len=%zu\n", str.length);
+        ok(strlen(str.data) == str.length, "len=%zu\n", str.length);

WineAStrFree(&str);
     }
@@ -59,25 +61,25 @@ static void test_init_append(void)
         ok(WineWStrInit(&str, MAX_PATH - 1) == S_OK, "@init\n");

/* string concat */
-        ok(WineWStrAppendW(&str, L"Hello world") == S_OK, "@append\n");
-        WineWStrAppendW(&str, L"Hello world");
-        WineWStrAppendW(&str, L"Hello world");
-        ok(str.length == 11 * 3, "len=%ld\n", str.length);
-        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+        ok(WineWStrAppend(&str, L"Hello world") == S_OK, "@append\n");
+        WineWStrAppend(&str, L"Hello world");
+        WineWStrAppend(&str, L"Hello world");
+        ok(str.length == 11 * 3, "len=%zu\n", str.length);
+        ok(lstrlenW(str.data) == str.length, "len=%zu\n", str.length);
         ok(str.capacity >= str.length, "cap=%ld\n", str.capacity);

/* resize without shrink */
         WineWStrResize(&str, str.length + 10);
-        ok(str.length == 11 * 3, "len=%ld\n", str.length);
-        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+        ok(str.length == 11 * 3, "len=%zu\n", str.length);
+        ok(lstrlenW(str.data) == str.length, "len=%zu\n", str.length);

/* resize with shrink */
         for (i = 3; i < 100; ++i)
-            WineWStrAppendW(&str, L"Hello world");
-        ok(str.length == 11 * 100, "len=%ld\n", str.length);
+            WineWStrAppend(&str, L"Hello world");
+        ok(str.length == 11 * 100, "len=%zu\n", str.length);
         WineWStrResize(&str, MAX_PATH - 1);
-        ok(str.length == MAX_PATH - 1, "len=%ld\n", str.length);
-        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+        ok(str.length == MAX_PATH - 1, "len=%zu\n", str.length);
+        ok(lstrlenW(str.data) == str.length, "len=%zu\n", str.length);

WineWStrFree(&str);
     }
@@ -91,18 +93,18 @@ static void test_replace_heap(void)
         char *new_heap;
         WINEASTR str;
         WineAStrInit(&str, alloc_count);
-        WineAStrAppendA(&str, "data to replace");
+        WineAStrAppend(&str, "data to replace");

new_heap = heap_alloc(alloc_count * sizeof(char));
         strcpy(new_heap, dummy_text_a);
         WineAStrReplaceHeap(&str, &new_heap);
         ok(new_heap == NULL, "p=%p\n", new_heap);
-        ok(str.length == strlen(dummy_text_a), "len=%ld\n", str.length);
+        ok(str.length == strlen(dummy_text_a), "len=%zu\n", str.length);
         ok(strcmp(str.data, dummy_text_a) == 0, "@strcmp\n");

new_heap = NULL;
         WineAStrReplaceHeap(&str, &new_heap);
-        ok(strlen(str.data) == 0, "len=%ld\n", str.length);
+        ok(strlen(str.data) == 0, "len=%zu\n", str.length);

WineAStrFree(&str);
     }
@@ -111,18 +113,18 @@ static void test_replace_heap(void)
         WCHAR *new_heap;
         WINEWSTR str;
         WineWStrInit(&str, alloc_count);
-        WineWStrAppendW(&str, L"data to replace");
+        WineWStrAppend(&str, L"data to replace");

new_heap = heap_alloc(alloc_count * sizeof(WCHAR));
         lstrcpyW(new_heap, dummy_text_w);
         WineWStrReplaceHeap(&str, &new_heap);
         ok(new_heap == NULL, "p=%p\n", new_heap);
-        ok(str.length == lstrlenW(dummy_text_w), "len=%ld\n", str.length);
+        ok(str.length == lstrlenW(dummy_text_w), "len=%zu\n", str.length);
         ok(lstrcmpW(str.data, dummy_text_w) == 0, "@lstrcmpW\n");

new_heap = NULL;
         WineWStrReplaceHeap(&str, &new_heap);
-        ok(lstrlenW(str.data) == 0, "len=%ld\n", str.length);
+        ok(lstrlenW(str.data) == 0, "len=%zu\n", str.length);

WineWStrFree(&str);
     }
@@ -135,13 +137,13 @@ static void test_append_convert(void)
         WINEASTR str;
         WineAStrInit(&str, 0);

-        WineAStrAppendW(&str, CP_ACP, dummy_text_w);
-        ok(strlen(str.data) == str.length, "len=%ld\n", str.length);
+        WineAStrAppendW(&str, CP_ACP, dummy_text_w, -1);
+        ok(strlen(str.data) == str.length, "len=%zu\n", str.length);
         ok(strcmp(str.data, dummy_text_a) == 0, "@strcmp\n");

/* repeat. double length. */
-        WineAStrAppendW(&str, CP_ACP, dummy_text_w);
-        ok(str.length == strlen(dummy_text_a) * 2, "len=%ld\n", str.length);
+        WineAStrAppendW(&str, CP_ACP, dummy_text_w, -1);
+        ok(str.length == strlen(dummy_text_a) * 2, "len=%zu\n", str.length);

WineAStrFree(&str);
     }
@@ -150,13 +152,47 @@ static void test_append_convert(void)
         WINEWSTR str;
         WineWStrInit(&str, 0);

-        WineWStrAppendA(&str, CP_ACP, dummy_text_a);
-        ok(lstrlenW(str.data) == str.length, "len=%ld\n", str.length);
+        WineWStrAppendA(&str, CP_ACP, dummy_text_a, -1);
+        ok(lstrlenW(str.data) == str.length, "len=%zu\n", str.length);
         ok(lstrcmpW(str.data, dummy_text_w) == 0, "@lstrcmpW\n");

/* repeat. double length. */
-        WineWStrAppendA(&str, CP_ACP, dummy_text_a);
-        ok(str.length == lstrlenW(dummy_text_w) * 2, "len=%ld\n", str.length);
+        WineWStrAppendA(&str, CP_ACP, dummy_text_a, -1);
+        ok(str.length == lstrlenW(dummy_text_w) * 2, "len=%zu\n", str.length);
+
+        WineWStrFree(&str);
+    }
+}
+
+static void test_append_limited(void)
+{
+    {
+        /* ANSI string: append with specified length */
+        WINEASTR str;
+        WineAStrInit(&str, 0);
+
+        ok(WineAStrAppendA(&str, dummy_text_a, 9) == S_OK, "@appendN\n");
+        ok(str.length == 9, "len=%zu\n", str.length);
+        ok(strcmp(str.data, dummy_text_a_part) == 0, "@strcmp\n");
+
+        ok(WineAStrAppendW(&str, CP_ACP, dummy_text_w, 9) == S_OK, "@appendN\n");
+        ok(str.length == 18, "len=%zu, %s\n", str.length, str.data);
+        ok(strcmp(str.data + 9, dummy_text_a_part) == 0, "@strcmp\n");
+
+        WineAStrFree(&str);
+    }
+    {
+        /* Wide string: append with specified length */
+        WINEWSTR str;
+        WineWStrInit(&str, 0);
+
+        ok(WineWStrAppendW(&str, dummy_text_w, 9) == S_OK, "@appendN\n");
+        ok(str.length == 9, "len=%zu\n", str.length);
+        ok(lstrcmpW(str.data, dummy_text_w_part) == 0, "@lstrcmpW\n");
+
+        ok(WineWStrAppendA(&str, CP_ACP, dummy_text_a, 9) == S_OK, "@appendN\n");
+        ok(str.length == 18, "len=%zu\n", str.length);
+        ok(lstrcmpW(str.data + 9, dummy_text_w_part) == 0, "@lstrcmpW\n");

WineWStrFree(&str);
     }
@@ -170,7 +206,7 @@ static void test_append_formatted(void)
         WineAStrInit(&str, 0);

ok(WineAStrAppendFmt(&str, "TEST %d %c+", 321, 'X') == S_OK, "@appendFmt\n");
-        ok(strcmp(str.data, "TEST 321 X+") == 0, "@strcmp, len=%ld\n", str.length);
+        ok(strcmp(str.data, "TEST 321 X+") == 0, "@strcmp, len=%zu\n", str.length);

WineAStrFree(&str);
     }
@@ -180,7 +216,7 @@ static void test_append_formatted(void)
         WineWStrInit(&str, 0);

ok(WineWStrAppendFmt(&str, L"TEST %d %c+", 321, L'X') == S_OK, "@appendFmt\n");
-        ok(lstrcmpW(str.data, L"TEST 321 X+") == 0, "@lstrcmpW, len=%ld\n", str.length);
+        ok(lstrcmpW(str.data, L"TEST 321 X+") == 0, "@lstrcmpW, len=%zu\n", str.length);

WineWStrFree(&str);
     }
@@ -191,5 +227,6 @@ START_TEST(winestring)
     test_init_append();
     test_replace_heap();
     test_append_convert();
+    test_append_limited();
     test_append_formatted();
 }
diff --git a/include/wine/string.h b/include/wine/string.h
index 15a76fcd2fa..66a834bd213 100644
--- a/include/wine/string.h
+++ b/include/wine/string.h
@@ -33,6 +33,9 @@
 #endif

+#define WineAStrAppend(str, data) WineAStrAppendA(str, data, -1)
+#define WineWStrAppend(str, data) WineWStrAppendW(str, data, -1)
+
 /** Dynamic container for ANSI strings */
 typedef struct _WINEASTR {
     char *data;      /**< C-string. The address may change due to realloc. */
@@ -41,7 +44,7 @@ typedef struct _WINEASTR {
 } WINEASTR;

/** One-time dynamic container initialization
- * @param capacity Amount of characters to preallocate
+ * @param capacity Amount of characters to preallocate. Higher values result in less reallocations.
  */
 static HRESULT WineAStrInit(WINEASTR *str, size_t capacity)
 {
@@ -76,13 +79,13 @@ static void WineAStrReplaceHeap(WINEASTR *str, char **src_data)
     }
 }

-/** Expand (realloc) or trim the container data
+/** Expand (realloc) or trim the container data. Doubles the specified capacity upon growth.
  * @param capacity Minimal amount of characters to hold.
  */
 static HRESULT WineAStrResize(WINEASTR *str, size_t capacity)
 {
     if (capacity > str->capacity) {
-        char *new_ptr = heap_realloc(str->data, (capacity + 1) * sizeof(char));
+        char *new_ptr = heap_realloc(str->data, (capacity * 2 + 1) * sizeof(char));
         if (!new_ptr)
             return E_OUTOFMEMORY;

@@ -96,7 +99,7 @@ static HRESULT WineAStrResize(WINEASTR *str, size_t capacity)
     return S_OK;
 }

-static HRESULT WineAStrAppendA(WINEASTR *str, const char *data)
+static HRESULT WineAStrAppendA(WINEASTR *str, const char *data, int n)
 {
     HRESULT hr;
     size_t len;
@@ -104,35 +107,43 @@ static HRESULT WineAStrAppendA(WINEASTR *str, const char *data)
     if (!data[0])
         return S_OK;

-    len = strlen(data);
-    hr = WineAStrResize(str, (str->length + len) * 2);
+    len = n < 0 ? strlen(data) : n;
+    hr = WineAStrResize(str, str->length + len);
     if (hr != S_OK)
         return hr;

-    memcpy(&str->data[str->length], data, (len + 1) * sizeof(char));
+    memcpy(&str->data[str->length], data, len * sizeof(char));
     str->length += len;
+    str->data[str->length] = 0;
     return S_OK;
 }

-/** Convert and append PWSTR (WCHAR*) */
-static HRESULT WineAStrAppendW(WINEASTR *str, UINT codepage, const WCHAR *data)
+/** Convert and append PWSTR (WCHAR*)
+ * @param n Amount of characters to convert. -1 to convert until a null character.is reached
+ */
+static HRESULT WineAStrAppendW(WINEASTR *str, UINT codepage, const WCHAR *data, int n)
 {
     HRESULT hr;
-    size_t len;
+    int len;

if (!data[0])
         return S_OK;

-    len = WideCharToMultiByte(codepage, 0, data, -1, NULL, 0, NULL, NULL);
-    hr = WineAStrResize(str, (str->length + len) * 2);
+    len = WideCharToMultiByte(codepage, 0, data, n, NULL, 0, NULL, NULL);
+    hr = WineAStrResize(str, str->length + len);
     if (hr != S_OK)
         return hr;

-    len = WideCharToMultiByte(codepage, 0, data, -1, &str->data[str->length], str->capacity + 1, NULL, NULL);
+    len = WideCharToMultiByte(codepage, 0, data, n, &str->data[str->length], str->capacity + 1, NULL, NULL);
     if (len <= 0)
         return HRESULT_FROM_WIN32(GetLastError());

+    /* The null character might or might not be written */
     str->length += len - 1;
+    if (str->data[str->length] != 0) {
+        str->length++;
+        str->data[str->length] = 0;
+    }
     return S_OK;
 }

@@ -144,13 +155,13 @@ static HRESULT WINAPIV __WINE_PRINTF_ATTR(2,3) WineAStrAppendFmt(WINEASTR *str,
     va_list valist;

va_start(valist, fmt);
-    len = vsprintf(NULL, fmt, valist);
+    len = _vscprintf(fmt, valist);
     if (len < 0) {
         hr = E_UNEXPECTED;
         goto end;
     }

-    hr = WineAStrResize(str, (str->length + len) * 2);
+    hr = WineAStrResize(str, str->length + len);
     if (hr != S_OK)
         goto end;

@@ -172,7 +183,7 @@ typedef struct _WINEWSTR {
 } WINEWSTR;

/** One-time dynamic container initialization
- * @param capacity Amount of characters to preallocate
+ * @param capacity Amount of characters to preallocate. Higher values result in less reallocations.
  */
 static HRESULT WineWStrInit(WINEWSTR *str, size_t capacity)
 {
@@ -207,13 +218,13 @@ static void WineWStrReplaceHeap(WINEWSTR *str, WCHAR **src_data)
     }
 }

-/** Expand (realloc) or trim the container data
+/** Expand (realloc) or trim the container data. Doubles the specified capacity upon growth.
  * @param capacity Minimal amount of characters to hold.
  */
 static HRESULT WineWStrResize(WINEWSTR *str, size_t capacity)
 {
     if (capacity > str->capacity) {
-        WCHAR *new_ptr = heap_realloc(str->data, (capacity + 1) * sizeof(WCHAR));
+        WCHAR *new_ptr = heap_realloc(str->data, (capacity * 2 + 1) * sizeof(WCHAR));
         if (!new_ptr)
             return E_OUTOFMEMORY;

@@ -227,7 +238,7 @@ static HRESULT WineWStrResize(WINEWSTR *str, size_t capacity)
     return S_OK;
 }

-static HRESULT WineWStrAppendW(WINEWSTR *str, const WCHAR *data)
+static HRESULT WineWStrAppendW(WINEWSTR *str, const WCHAR *data, int n)
 {
     HRESULT hr;
     size_t len;
@@ -235,35 +246,43 @@ static HRESULT WineWStrAppendW(WINEWSTR *str, const WCHAR *data)
     if (!data[0])
         return S_OK;

-    len = lstrlenW(data);
-    hr = WineWStrResize(str, (str->length + len) * 2);
+    len = n < 0 ? lstrlenW(data) : n;
+    hr = WineWStrResize(str, str->length + len);
     if (hr != S_OK)
         return hr;

-    memcpy(&str->data[str->length], data, (len + 1) * sizeof(WCHAR));
+    memcpy(&str->data[str->length], data, len * sizeof(WCHAR));
     str->length += len;
+    str->data[str->length] = 0;
     return S_OK;
 }

-/** Convert and append PSTR (char*) */
-static HRESULT WineWStrAppendA(WINEWSTR *str, UINT codepage, const char *data)
+/** Convert and append PSTR (char*)
+ * @param n Amount of characters to convert. -1 to convert until a null character.is reached
+ */
+static HRESULT WineWStrAppendA(WINEWSTR *str, UINT codepage, const char *data, int n)
 {
     HRESULT hr;
-    size_t len;
+    int len;

if (!data[0])
         return S_OK;

-    len = MultiByteToWideChar(codepage, 0, data, -1, NULL, 0);
-    hr = WineWStrResize(str, (str->length + len) * 2);
+    len = MultiByteToWideChar(codepage, 0, data, n, NULL, 0);
+    hr = WineWStrResize(str, str->length + len);
     if (hr != S_OK)
         return hr;

-    len = MultiByteToWideChar(codepage, 0, data, -1, &str->data[str->length], str->capacity + 1);
+    len = MultiByteToWideChar(codepage, 0, data, n, &str->data[str->length], str->capacity + 1);
     if (len <= 0)
         return HRESULT_FROM_WIN32(GetLastError());

+    /* The null character might or might not be written */
     str->length += len - 1;
+    if (str->data[str->length] != 0) {
+        str->length++;
+        str->data[str->length] = 0;
+    }
     return S_OK;
 }

@@ -275,13 +294,13 @@ static HRESULT WINAPIV WineWStrAppendFmt(WINEWSTR *str, const WCHAR *fmt, ...)
     va_list valist;

va_start(valist, fmt);
-    len = vswprintf(NULL, -1, fmt, valist);
+    len = _vscwprintf(fmt, valist);
     if (len < 0) {
         hr = E_UNEXPECTED;
         goto end;
     }

-    hr = WineWStrResize(str, (str->length + len) * 2);
+    hr = WineWStrResize(str, str->length + len);
     if (hr != S_OK)
         goto end;

[Piotr Caban (＠piotr)]

On Fri Sep 23 22:08:21 2022 +0000, Stefan Rentsch wrote:
> What exactly makes a generic implementation *not* beneficial in
> long-term? Currently there are least the following places that use
> dynamic string containers in private helper functions:
> - atl/registrar.c: https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/dlls/atl/registrar.c#L6...
> - jscript/json.c: https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/dlls/jscript/json.c#L36...
> - mshtml/range.c: https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/dlls/mshtml/range.c#L35...
> - ntdll/unix/env.c:
>
> https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/dlls/ntdll/unix/env.c#L...
> (dynamic string, no container)
> - vbscript/vbregexp.c: https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/dlls/vbscript/vbregexp....
> - wineps.drv/type1.c: https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/dlls/wineps.drv/type1.c...
> - winex11.drv/clipboard.c:
>
> https://gitlab.winehq.org/wine/wine/-/blob/wine-7.18/dlls/winex11.drv/clipbo...
> (dynamic string, no container) 
> Each implementation seems to be written from scratch, introducing code
> duplication. I could copy & paste a few functions for shell32 to ensure
> safe concat and bounds checks, which however again introduces more code
> to maintain, if that's the preferred practice.
> EDIT: I am open to suggestions. Please let me know if I can do something.

I think it's easy to implement helpers in modules that need it so there's no big profit from adding global header. It also resolves a problem of e.g. using different allocators in different modules and allows optimizing helpers for specific use cases. That's why I think it's better to introduce string helper functions in shell32 if needed.

[Stefan Rentsch (＠restet)]

On Mon Sep 26 14:44:18 2022 +0000, Piotr Caban wrote:
> I think it's easy to implement helpers in modules that need it so
> there's no big profit from adding global header. It also resolves a
> problem of e.g. using different allocators in different modules and
> allows optimizing helpers for specific use cases. That's why I think
> it's better to introduce string helper functions in shell32 if needed.

The allocator problem could be solved using defines prior to the header inclusion, but I see that it would get a bit messy to work with. The code duplication will not end, but I suppose that's not my problem. ¯\_(ツ)_/¯

Thank you for your input and having a look at this MR in first place. I will add module-specific functions in that case.

[Stefan Rentsch (＠restet)]

On Sat Sep 24 21:20:35 2022 +0000, **** wrote:
> Marvin replied on the mailing list:
> Hi,
> It looks like your patch introduced the new failures shown below.
> Please investigate and fix them before resubmitting your patch.
> If they are not new, fixing them anyway would help a lot. Otherwise
> please ask for the known failures list to be updated.
> The full results can be found at:
> https://testbot.winehq.org/JobDetails.pl?Key=124150
> Your paranoid android.
> === w10pro64_zh_CN (64 bit report) ===
> msvcrt:
> file.c:1743: Test failed: handle = 000000000000004C
> file.c:1747: Test failed: info->handle = 000000000000004C
> file.c:1750: Test failed: stdin->_file = 0
> file.c:1755: Test failed: errno = 22
> file.c:1765: Test failed: errno = 22
> file.c:1785: Test failed: errno = 22
> file.c:1789: Test failed: fclose(stdin) returned 0
> file.c:1790: Test failed: errno = -559038737
> === w7u_2qxl (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w7u_adm (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w7u_el (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w8 (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w8adm (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w864 (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064v1507 (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064v1809 (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064 (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064_tsign (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w10pro64 (32 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w864 (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064v1507 (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064v1809 (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064 (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064_2qxl (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064_adm (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w1064_tsign (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w10pro64 (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w10pro64_en_AE_u8 (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w10pro64_ar (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w10pro64_ja (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0
> === w10pro64_zh_CN (64 bit report) ===
> msvcrt:
> winestring.c:172: Test failed: @appendFmt
> winestring.c:173: Test failed: @strcmp, len=0
> winestring.c:182: Test failed: @appendFmt
> winestring.c:183: Test failed: @lstrcmpW, len=0

The build did not include all commits. You are drunk.

[PATCH v2 0/2] MR910: msvcrt: Implement dynamic string container - closed

[Stefan Rentsch (＠restet)]

This merge request was closed by Stefan Rentsch.