[ros-diffs] [reactos] 01/01: [MSVCRT][CRT_APITEST] Implement _wsystem (#5032)

[Katayama Hirofumi MZ]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=f172503d57236c8388b33…

commit f172503d57236c8388b334bc5ecef02ea34d4561
Author:     Katayama Hirofumi MZ <katayama.hirofumi.mz(a)gmail.com>
AuthorDate: Sun Mar 5 21:01:14 2023 +0900
Commit:     GitHub <noreply(a)github.com>
CommitDate: Sun Mar 5 21:01:14 2023 +0900

    [MSVCRT][CRT_APITEST] Implement _wsystem (#5032)
    
    Implement _wsystem(), by referring system().
    Improve system().
    Use WaitForSingleObject in system() and _wsystem().
    Check existence of COMSPEC.
    Thanks ChatGPT.
---
 modules/rostests/apitests/crt/_wsystem.c           |  72 +++++++++++
 .../rostests/apitests/crt/crtdll_crt_apitest.cmake |   2 +-
 .../rostests/apitests/crt/msvcrt_crt_apitest.cmake |   4 +-
 modules/rostests/apitests/crt/system.c             |  72 +++++++++++
 modules/rostests/apitests/crt/testlist.c           |   8 ++
 sdk/lib/crt/process/_system.c                      | 144 ++++++++++++++-------
 6 files changed, 255 insertions(+), 47 deletions(-)

diff --git a/modules/rostests/apitests/crt/_wsystem.c
b/modules/rostests/apitests/crt/_wsystem.c
new file mode 100644
index 00000000000..1659b3ee609
--- /dev/null
+++ b/modules/rostests/apitests/crt/_wsystem.c
@@ -0,0 +1,72 @@
+/*
+ * PROJECT:     ReactOS CRT
+ * LICENSE:     MIT (https://spdx.org/licenses/MIT)
+ * PURPOSE:     Tests for _wsystem()
+ * COPYRIGHT:   Copyright 2023 Katayama Hirofumi MZ
<katayama.hirofumi.mz(a)gmail.com>
+ */
+
+#include <apitest.h>
+#include <apitest_guard.h>
+
+START_TEST(_wsystem)
+{
+    int ret;
+    WCHAR szCmdExe[MAX_PATH];
+
+    GetSystemDirectoryW(szCmdExe, _countof(szCmdExe));
+    lstrcatW(szCmdExe, L"\\cmd.exe");
+
+    SetEnvironmentVariableW(L"COMSPEC", NULL);
+    errno = 0xDEADBEEF;
+    ret = _wsystem(NULL);
+    ok_int(errno, 0xDEADBEEF);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableW(L"COMSPEC", L"InvalidComSpec");
+    errno = 0xDEADBEEF;
+    ret = _wsystem(NULL);
+    ok_int(errno, 0xDEADBEEF);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableW(L"COMSPEC", szCmdExe);
+    errno = 0xDEADBEEF;
+    ret = _wsystem(NULL);
+    ok_int(errno, 0xDEADBEEF);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableW(L"COMSPEC", NULL);
+    errno = 0xDEADBEEF;
+    ret = _wsystem(L"echo This is a test");
+    ok_int(errno, 0);
+    ok_int(ret, 0);
+
+    SetEnvironmentVariableW(L"COMSPEC", L"InvalidComSpec");
+    errno = 0xDEADBEEF;
+    ret = _wsystem(L"echo This is a test");
+    ok_int(errno, 0);
+    ok_int(ret, 0);
+
+    SetEnvironmentVariableW(L"COMSPEC", szCmdExe);
+    errno = 0xDEADBEEF;
+    ret = _wsystem(L"echo This is a test");
+    ok_int(errno, 0);
+    ok_int(ret, 0);
+
+    SetEnvironmentVariableW(L"COMSPEC", NULL);
+    errno = 0xDEADBEEF;
+    ret = _wsystem(L"InvalidCommandLine");
+    ok_int(errno, 0);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableW(L"COMSPEC", L"InvalidComSpec");
+    errno = 0xDEADBEEF;
+    ret = _wsystem(L"InvalidCommandLine");
+    ok_int(errno, 0);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableW(L"COMSPEC", szCmdExe);
+    errno = 0xDEADBEEF;
+    ret = _wsystem(L"InvalidCommandLine");
+    ok_int(errno, 0);
+    ok_int(ret, 1);
+}
diff --git a/modules/rostests/apitests/crt/crtdll_crt_apitest.cmake
b/modules/rostests/apitests/crt/crtdll_crt_apitest.cmake
index f40c603c480..c6f60ea1d56 100644
--- a/modules/rostests/apitests/crt/crtdll_crt_apitest.cmake
+++ b/modules/rostests/apitests/crt/crtdll_crt_apitest.cmake
@@ -480,7 +480,7 @@ list(APPEND SOURCE_CRTDLL
 #    strxfrm.c
 #    swprintf.c
 #    swscanf.c
-#    system.c
+    system.c
 #    tan.c
 #    tanh.c
 #    time.c
diff --git a/modules/rostests/apitests/crt/msvcrt_crt_apitest.cmake
b/modules/rostests/apitests/crt/msvcrt_crt_apitest.cmake
index 4677f90fbfa..919a7b01b64 100644
--- a/modules/rostests/apitests/crt/msvcrt_crt_apitest.cmake
+++ b/modules/rostests/apitests/crt/msvcrt_crt_apitest.cmake
@@ -981,7 +981,7 @@ list(APPEND SOURCE_MSVCRT
 #    _wstrdate_s
 #    _wstrtime.c
 #    _wstrtime_s
-#    _wsystem.c
+    _wsystem.c
 #    _wtempnam.c
 #    _wtempnam_dbg
 #    _wtmpnam.c
@@ -1189,7 +1189,7 @@ list(APPEND SOURCE_MSVCRT
 #    swprintf_s.c
 #    swscanf.c
 #    swscanf_s.c
-#    system.c
+    system.c
 #    tan.c
 #    tanh.c
 #    time.c
diff --git a/modules/rostests/apitests/crt/system.c
b/modules/rostests/apitests/crt/system.c
new file mode 100644
index 00000000000..5d49a0babf1
--- /dev/null
+++ b/modules/rostests/apitests/crt/system.c
@@ -0,0 +1,72 @@
+/*
+ * PROJECT:     ReactOS CRT
+ * LICENSE:     MIT (https://spdx.org/licenses/MIT)
+ * PURPOSE:     Tests for system()
+ * COPYRIGHT:   Copyright 2023 Katayama Hirofumi MZ
&lt;katayama.hirofumi.mz(a)gmail.com&gt;
+ */
+
+#include <apitest.h>
+#include <apitest_guard.h>
+
+START_TEST(system)
+{
+    int ret;
+    CHAR szCmdExe[MAX_PATH];
+
+    GetSystemDirectoryA(szCmdExe, _countof(szCmdExe));
+    lstrcatA(szCmdExe, "\\cmd.exe");
+
+    SetEnvironmentVariableA("COMSPEC", NULL);
+    errno = 0xDEADBEEF;
+    ret = system(NULL);
+    ok_int(errno, 0xDEADBEEF);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableA("COMSPEC", "InvalidComSpec");
+    errno = 0xDEADBEEF;
+    ret = system(NULL);
+    ok_int(errno, 0xDEADBEEF);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableA("COMSPEC", szCmdExe);
+    errno = 0xDEADBEEF;
+    ret = system(NULL);
+    ok_int(errno, 0xDEADBEEF);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableA("COMSPEC", NULL);
+    errno = 0xDEADBEEF;
+    ret = system("echo This is a test");
+    ok_int(errno, 0);
+    ok_int(ret, 0);
+
+    SetEnvironmentVariableA("COMSPEC", "InvalidComSpec");
+    errno = 0xDEADBEEF;
+    ret = system("echo This is a test");
+    ok_int(errno, 0);
+    ok_int(ret, 0);
+
+    SetEnvironmentVariableA("COMSPEC", szCmdExe);
+    errno = 0xDEADBEEF;
+    ret = system("echo This is a test");
+    ok_int(errno, 0);
+    ok_int(ret, 0);
+
+    SetEnvironmentVariableA("COMSPEC", NULL);
+    errno = 0xDEADBEEF;
+    ret = system("InvalidCommandLine");
+    ok_int(errno, 0);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableA("COMSPEC", "InvalidComSpec");
+    errno = 0xDEADBEEF;
+    ret = system("InvalidCommandLine");
+    ok_int(errno, 0);
+    ok_int(ret, 1);
+
+    SetEnvironmentVariableA("COMSPEC", szCmdExe);
+    errno = 0xDEADBEEF;
+    ret = system("InvalidCommandLine");
+    ok_int(errno, 0);
+    ok_int(ret, 1);
+}
diff --git a/modules/rostests/apitests/crt/testlist.c
b/modules/rostests/apitests/crt/testlist.c
index fd746d39011..14f0bb97ad3 100644
--- a/modules/rostests/apitests/crt/testlist.c
+++ b/modules/rostests/apitests/crt/testlist.c
@@ -35,10 +35,12 @@ extern void func_strcpy(void);
 extern void func_strlen(void);
 extern void func_strnlen(void);
 extern void func_strtoul(void);
+extern void func_system(void);
 extern void func_wcsnlen(void);
 extern void func_wcstombs(void);
 extern void func_wcstoul(void);
 extern void func_wctomb(void);
+extern void func__wsystem(void);
 extern void func___getmainargs(void);
 
 extern void func_static_construct(void);
@@ -57,6 +59,12 @@ const struct test winetest_testlist[] =
     { "strcpy", func_strcpy },
     { "strlen", func_strlen },
     { "strtoul", func_strtoul },
+#if defined(TEST_CRTDLL) || defined(TEST_MSVCRT)
+    { "system", func_system },
+#endif
+#if defined(TEST_MSVCRT)
+    { "_wsystem", func__wsystem },
+#endif
     { "wcstoul", func_wcstoul },
     { "wctomb", func_wctomb },
     { "wcstombs", func_wcstombs },
diff --git a/sdk/lib/crt/process/_system.c b/sdk/lib/crt/process/_system.c
index 6d879fce591..6ce5b117808 100644
--- a/sdk/lib/crt/process/_system.c
+++ b/sdk/lib/crt/process/_system.c
@@ -4,6 +4,7 @@
  * FILE:        lib/sdk/crt/process/_system.c
  * PURPOSE:     Excutes a shell command
  * PROGRAMER:   Ariadne
+ *              Katayama Hirofumi MZ
  * UPDATE HISTORY:
  *              04/03/99: Created
  */
@@ -24,70 +25,48 @@ int system(const char *command)
 
   PROCESS_INFORMATION ProcessInformation;
   STARTUPINFOA StartupInfo;
-  char *s;
   BOOL result;
-
-  int nStatus;
+  DWORD exit_code;
+  char cmd_exe[MAX_PATH];
 
   szComSpec = getenv("COMSPEC");
 
 // system should return 0 if command is null and the shell is found
 
   if (command == NULL) {
-    if (szComSpec == NULL)
-      return 0;
-    else
-      return 1;
+    return (szComSpec == NULL) ? 0 : 1;
   }
 
-  if (szComSpec == NULL)
-    return -1;
-
-// should return 127 or 0 ( MS ) if the shell is not found
-// _set_errno(ENOENT);
-
-  if (szComSpec == NULL)
+  if (!szComSpec || GetFileAttributesA(szComSpec) == INVALID_FILE_ATTRIBUTES)
   {
-    szComSpec = "cmd.exe";
+    GetSystemDirectoryA(cmd_exe, _countof(cmd_exe));
+    strcat(cmd_exe, "\\cmd.exe");
+    szComSpec = cmd_exe;
   }
 
-  /* split the path from shell command */
-  s = max(strrchr(szComSpec, '\\'), strrchr(szComSpec, '/'));
-  if (s == NULL)
-    s = szComSpec;
-  else
-    s++;
-
-  szCmdLine = malloc(strlen(s) + 4 + strlen(command) + 1);
+  szCmdLine = LocalAlloc(LPTR, 1 + strlen(szComSpec) + 5 + strlen(command) + 1);
   if (szCmdLine == NULL)
   {
-     _set_errno(ENOMEM);
+     _dosmaperr(GetLastError());
      return -1;
   }
 
-  strcpy(szCmdLine, s);
-  s = strrchr(szCmdLine, '.');
-  if (s)
-    *s = 0;
-  strcat(szCmdLine, " /C ");
+  strcpy(szCmdLine, "\"");
+  strcat(szCmdLine, szComSpec);
+  strcat(szCmdLine, "\" /C ");
   strcat(szCmdLine, command);
 
 //command file has invalid format ENOEXEC
 
-  memset (&StartupInfo, 0, sizeof(StartupInfo));
+  memset(&StartupInfo, 0, sizeof(StartupInfo));
   StartupInfo.cb = sizeof(StartupInfo);
-  StartupInfo.lpReserved= NULL;
   StartupInfo.dwFlags = STARTF_USESHOWWINDOW;
   StartupInfo.wShowWindow = SW_SHOWDEFAULT;
-  StartupInfo.lpReserved2 = NULL;
-  StartupInfo.cbReserved2 = 0;
-
-// According to ansi standards the new process should ignore  SIGINT and SIGQUIT
-// In order to disable ctr-c the process is created with CREATE_NEW_PROCESS_GROUP,
-// thus SetConsoleCtrlHandler(NULL,TRUE) is made on behalf of the new process.
 
+// In order to disable Ctrl+C, the process is created with CREATE_NEW_PROCESS_GROUP.
+// Thus, SetConsoleCtrlHandler(NULL, TRUE) is made on behalf of the new process.
 
-//SIGCHILD should be blocked aswell
+//SIGCHILD should be blocked as well
 
   result = CreateProcessA(szComSpec,
 	                  szCmdLine,
@@ -99,7 +78,7 @@ int system(const char *command)
 			  NULL,
 			  &StartupInfo,
 			  &ProcessInformation);
-  free(szCmdLine);
+  LocalFree(szCmdLine);
 
   if (result == FALSE)
   {
@@ -109,15 +88,92 @@ int system(const char *command)
 
   CloseHandle(ProcessInformation.hThread);
 
-// system should wait untill the calling process is finished
-  _cwait(&nStatus,(intptr_t)ProcessInformation.hProcess,0);
+  /* Wait for the process to exit */
+  WaitForSingleObject(ProcessInformation.hProcess, INFINITE);
+  GetExitCodeProcess(ProcessInformation.hProcess, &exit_code);
+
   CloseHandle(ProcessInformation.hProcess);
 
-  return nStatus;
+  _set_errno(0);
+  return (int)exit_code;
 }
 
 int CDECL _wsystem(const wchar_t* cmd)
 {
-    FIXME("_wsystem stub\n");
-    return -1;
+    wchar_t *cmdline = NULL;
+    wchar_t *comspec = NULL;
+    PROCESS_INFORMATION process_info;
+    STARTUPINFOW startup_info;
+    BOOL result;
+    DWORD exit_code;
+    wchar_t cmd_exe[MAX_PATH];
+
+    comspec = _wgetenv(L"COMSPEC");
+
+    /* _wsystem should return 0 if cmd is null and the shell is found */
+    if (cmd == NULL)
+    {
+        return (comspec == NULL) ? 0 : 1;
+    }
+
+    if (comspec == NULL || GetFileAttributesW(comspec) == INVALID_FILE_ATTRIBUTES)
+    {
+        GetSystemDirectoryW(cmd_exe, _countof(cmd_exe));
+        wcscat(cmd_exe, L"\\cmd.exe");
+        comspec = cmd_exe;
+    }
+
+    cmdline = LocalAlloc(LPTR, (1 + wcslen(comspec) + 5 + wcslen(cmd) + 1) *
sizeof(wchar_t));
+    if (cmdline == NULL)
+    {
+        _dosmaperr(GetLastError());
+        return -1;
+    }
+
+    wcscpy(cmdline, L"\"");
+    wcscat(cmdline, comspec);
+    wcscat(cmdline, L"\" /C ");
+    wcscat(cmdline, cmd);
+
+    /* command file has invalid format ENOEXEC */
+
+    memset(&startup_info, 0, sizeof(startup_info));
+    startup_info.cb = sizeof(startup_info);
+    startup_info.dwFlags = STARTF_USESHOWWINDOW;
+    startup_info.wShowWindow = SW_SHOWDEFAULT;
+
+    /* In order to disable Ctrl+C, the process is created with CREATE_NEW_PROCESS_GROUP.
+       Thus, SetConsoleCtrlHandler(NULL, TRUE) is made on behalf of the new process. */
+
+    /* SIGCHILD should be blocked as well */
+
+    /* Create the process to execute the command */
+    result = CreateProcessW(comspec,
+                            cmdline,
+                            NULL,
+                            NULL,
+                            TRUE,
+                            CREATE_NEW_PROCESS_GROUP,
+                            NULL,
+                            NULL,
+                            &startup_info,
+                            &process_info);
+    LocalFree(cmdline);
+
+    if (!result)
+    {
+        _dosmaperr(GetLastError());
+        return -1;
+    }
+
+    CloseHandle(process_info.hThread);
+
+    /* Wait for the process to exit */
+    WaitForSingleObject(process_info.hProcess, INFINITE);
+    GetExitCodeProcess(process_info.hProcess, &exit_code);
+
+    CloseHandle(process_info.hProcess);
+
+    _set_errno(0);
+    return (int)exit_code;
 }