msvcrt: Fix strtok_s and _mbstok_s for empty strings. (v2)

Bernhard Übelacker bernhardu at mailbox.org
Thu Aug 18 04:06:45 CDT 2016


Medical images (DICOM) processed with Merge eFilm Workstation are put
to DVD bundled with a standalone viewer eFilm Lite.

https://appdb.winehq.org/objectManager.php?sClass=application&iId=17270

The DVD, I got asked if it could be run in Linux, contains:
  Version 3.4.0 (Build 10)
  Built: Dec  2 2010 04:55:23

First it looks promising, but when progressbar reaches the end it crashes:
Backtrace:
=>0 0x7b43b69c RaiseException+0x8c(...) [...] in kernel32 (0x0033e118)
  1 0x7e0a8f19 MSVCRT__invalid_parameter+0x78() in msvcr80 (0x0033e168)
  2 0x7e0e4590 MSVCRT_strtok_s+0x27f(...) [...] in msvcr80 (0x0033e1c8)
  3 0x00454c8d in efilmlt (+0x54c8c) (0x0033e238)
  4 0x0045cafc in efilmlt (+0x5cafb) (0x064d4eec)
  5 0x064d5378 (0x782b8d2c)

This happens because strtok_s is first called with zero length string
and null context.
The second call is made with null string and unmodified context.

Therefore MSVCRT__invalid_parameter is called closing down the application.

With this change applied, the application is running well, and I could not
find any other issue in a short test.

As msvcrt.dll does not seem to export _set_invalid_parameter_handler,
and msvcr80 does currently not contain tests,
the test is added to msvcr90/tests.

v1:
https://www.winehq.org/pipermail/wine-patches/2016-August/153652.html
https://www.winehq.org/pipermail/wine-devel/2016-August/114394.html

Changes since v1:
- Fix signedness warnings in test__mbstok_s
- Remove own invalid parameter handler.

Signed-off-by: Bernhard Übelacker <bernhardu at mailbox.org>
---
 dlls/msvcr90/tests/msvcr90.c | 98 ++++++++++++++++++++++++++++++++++++++++++++
 dlls/msvcrt/mbcs.c           |  3 ++
 dlls/msvcrt/string.c         |  3 ++
 3 files changed, 104 insertions(+)

diff --git a/dlls/msvcr90/tests/msvcr90.c b/dlls/msvcr90/tests/msvcr90.c
index ecf25ec..d6d4e61 100644
--- a/dlls/msvcr90/tests/msvcr90.c
+++ b/dlls/msvcr90/tests/msvcr90.c
@@ -92,8 +92,10 @@ static int* (__cdecl *p_errno)(void);
 static __msvcrt_ulong* (__cdecl *p_doserrno)(void);
 static void (__cdecl *p_srand)(unsigned int);
 static char* (__cdecl *p_strtok)(char*, const char*);
+static char* (__cdecl *p_strtok_s)(char*, const char*, char**);
 static wchar_t* (__cdecl *p_wcstok)(wchar_t*, const wchar_t*);
 static unsigned char* (__cdecl *p__mbstok)(unsigned char*, const unsigned char*);
+static unsigned char* (__cdecl *p__mbstok_s)(unsigned char*, const unsigned char*, unsigned char**);
 static char* (__cdecl *p_strerror)(int);
 static wchar_t* (__cdecl *p_wcserror)(int);
 static char* (__cdecl *p_tmpnam)(char*);
@@ -355,8 +357,10 @@ static BOOL init(void)
     SET(p_doserrno, "__doserrno");
     SET(p_srand, "srand");
     SET(p_strtok, "strtok");
+    SET(p_strtok_s, "strtok_s");
     SET(p_wcstok, "wcstok");
     SET(p__mbstok, "_mbstok");
+    SET(p__mbstok_s, "_mbstok_s");
     SET(p_strerror, "strerror");
     SET(p_wcserror, "_wcserror");
     SET(p_tmpnam, "tmpnam");
@@ -1547,6 +1551,98 @@ static void test_mbstowcs(void)
     p_setlocale(LC_ALL, "C");
 }
 
+static void test_strtok_s(void)
+{
+    char test[] = "a/b";
+    char empty[] = "";
+    char space[] = " ";
+    char delim[] = "/";
+    char *strret;
+    char *context;
+
+    if(!p_strtok_s) {
+        win_skip("strtok_s not found.\n");
+        return;
+    }
+
+    context = (char*)0xdeadbeef;
+    strret = p_strtok_s( test, delim, &context);
+    ok(strret == test, "Expected test, got %p.\n", strret);
+    ok(context == test+2, "Expected test+2, got %p.\n", context);
+
+    strret = p_strtok_s( NULL, delim, &context);
+    ok(strret == test+2, "Expected test+2, got %p.\n", strret);
+    ok(context == test+3, "Expected test+3, got %p.\n", context);
+
+    strret = p_strtok_s( NULL, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == test+3, "Expected test+3, got %p.\n", context);
+
+    context = NULL;
+    strret = p_strtok_s( empty, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == empty, "Expected empty, got %p.\n", context);
+
+    strret = p_strtok_s( NULL, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == empty, "Expected empty, got %p.\n", context);
+
+    context = NULL;
+    strret = p_strtok_s( space, delim, &context);
+    ok(strret == space, "Expected space, got %p.\n", strret);
+    ok(context == space+1, "Expected space+1, got %p.\n", context);
+
+    strret = p_strtok_s( NULL, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == space+1, "Expected space+1, got %p.\n", context);
+}
+
+static void test__mbstok_s(void)
+{
+    unsigned char test[] = "a/b";
+    unsigned char empty[] = "";
+    unsigned char space[] = " ";
+    unsigned char delim[] = "/";
+    unsigned char *strret;
+    unsigned char *context;
+
+    if(!p__mbstok_s) {
+        win_skip("strtok_s not found.\n");
+        return;
+    }
+
+    context = (unsigned char*)0xdeadbeef;
+    strret = p__mbstok_s( test, delim, &context);
+    ok(strret == test, "Expected test, got %p.\n", strret);
+    ok(context == test+2, "Expected test+2, got %p.\n", context);
+
+    strret = p__mbstok_s( NULL, delim, &context);
+    ok(strret == test+2, "Expected test+2, got %p.\n", strret);
+    ok(context == test+3, "Expected test+3, got %p.\n", context);
+
+    strret = p__mbstok_s( NULL, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == test+3, "Expected test+3, got %p.\n", context);
+
+    context = NULL;
+    strret = p__mbstok_s( empty, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == empty, "Expected empty, got %p.\n", context);
+
+    strret = p__mbstok_s( NULL, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == empty, "Expected empty, got %p.\n", context);
+
+    context = NULL;
+    strret = p__mbstok_s( space, delim, &context);
+    ok(strret == space, "Expected space, got %p.\n", strret);
+    ok(context == space+1, "Expected space+1, got %p.\n", context);
+
+    strret = p__mbstok_s( NULL, delim, &context);
+    ok(strret == NULL, "Expected NULL, got %p.\n", strret);
+    ok(context == space+1, "Expected space+1, got %p.\n", context);
+}
+
 START_TEST(msvcr90)
 {
     if(!init())
@@ -1576,4 +1672,6 @@ START_TEST(msvcr90)
     test_is_exception_typeof();
     test__AdjustPointer();
     test_mbstowcs();
+    test_strtok_s();
+    test__mbstok_s();
 }
diff --git a/dlls/msvcrt/mbcs.c b/dlls/msvcrt/mbcs.c
index fe88cc7..93bb1a7 100644
--- a/dlls/msvcrt/mbcs.c
+++ b/dlls/msvcrt/mbcs.c
@@ -1205,7 +1205,10 @@ unsigned char* CDECL _mbstok_s_l(unsigned char *str, const unsigned char *delim,
     while((c=_mbsnextc(str)) && _mbschr(delim, c))
         str += c>255 ? 2 : 1;
     if(!*str)
+    {
+        *ctx = str;
         return NULL;
+    }
 
     *ctx = str + (c>255 ? 2 : 1);
     while((c=_mbsnextc(*ctx)) && !_mbschr(delim, c))
diff --git a/dlls/msvcrt/string.c b/dlls/msvcrt/string.c
index 85f09ad..3e765d0 100644
--- a/dlls/msvcrt/string.c
+++ b/dlls/msvcrt/string.c
@@ -274,7 +274,10 @@ char * CDECL MSVCRT_strtok_s(char *str, const char *delim, char **ctx)
     while(*str && strchr(delim, *str))
         str++;
     if(!*str)
+    {
+        *ctx = str;
         return NULL;
+    }
 
     *ctx = str+1;
     while(**ctx && !strchr(delim, **ctx))
-- 
2.1.4




More information about the wine-patches mailing list