Commit: patch 9.1.2013: tests: Test_terminal_shell_option fails with conpty

1 view
Skip to first unread message

Christian Brabandt

unread,
3:31 PM (5 hours ago) 3:31 PM
to vim...@googlegroups.com
patch 9.1.2013: tests: Test_terminal_shell_option fails with conpty

Commit: https://github.com/vim/vim/commit/90e17110c476f38ae32449796b65e2a3a62a2827
Author: Muraoka Taro <koron....@gmail.com>
Date: Tue Dec 23 20:22:54 2025 +0000

patch 9.1.2013: tests: Test_terminal_shell_option fails with conpty

Problem: tests: When opening a conpty terminal, if process startup
fails, it will silently exit. As a result, the
Test_terminal_shell_option in test_terminal3.vim failed in
conpty.

In a winpty terminal, the winpty-provided error message
"CreateProcess failed" was displayed. The test is designed to
catch this error as an exception.

Solution: Make conpty fail with an error messages in the same way as winpty.
(Muraoka Taro)

In addition, since the GetWin32Error() function can obtain more detailed
error messages, the format has been changed to "CreateProcess failed:
{localized message from the OS}" for conpty.

Also, since the GetWin32Error() function returns errors in ACP (Active
Code Page) encoding, these have been converted to Vim's internal
encoding, enc. This will prevent messages from being garbled in
Japanese environments, etc. The output of this function was basically
used by the semsg() function in other places, so this change also fixes
potential similar garbled characters.

The test now errors out immediately in places where it is expected not
to be reached, and comments have been added about the expected content
of the winpty and conpty error messages.

closes: #18998

Signed-off-by: Muraoka Taro <koron....@gmail.com>
Signed-off-by: Christian Brabandt <c...@256bit.org>

diff --git a/src/os_win32.c b/src/os_win32.c
index ede7a15b2..8291f45b1 100644
--- a/src/os_win32.c
+++ b/src/os_win32.c
@@ -9074,22 +9074,38 @@ resize_console_buf(void)
char *
GetWin32Error(void)
{
- static char *oldmsg = NULL;
- char *msg = NULL;
+ static char *oldmsg = NULL;
+ char *acp_msg = NULL;
+ DWORD acp_len;
+ char_u *enc_msg = NULL;
+ int enc_len = 0;
+
+ // get formatted message from OS
+ acp_len = FormatMessage(
+ FORMAT_MESSAGE_ALLOCATE_BUFFER|FORMAT_MESSAGE_FROM_SYSTEM,
+ NULL, GetLastError(), 0, (LPSTR)&acp_msg, 0, NULL);
+ if (acp_len == 0 || acp_msg == NULL)
+ return NULL;

- FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER|FORMAT_MESSAGE_FROM_SYSTEM,
- NULL, GetLastError(), 0, (LPSTR)&msg, 0, NULL);
+ // clean oldmsg if remained.
if (oldmsg != NULL)
- LocalFree(oldmsg);
- if (msg == NULL)
+ {
+ vim_free(oldmsg);
+ oldmsg = NULL;
+ }
+
+ acp_to_enc(acp_msg, (int)acp_len, &enc_msg, &enc_len);
+ LocalFree(acp_msg);
+ if (enc_msg == NULL)
return NULL;

// remove trailing

- char *pcrlf = strstr(msg, "
");
+ char *pcrlf = strstr(enc_msg, "
");
if (pcrlf != NULL)
*pcrlf = NUL;
- oldmsg = msg;
- return msg;
+
+ oldmsg = enc_msg;
+ return enc_msg;
}

#if defined(FEAT_RELTIME)
diff --git a/src/terminal.c b/src/terminal.c
index 48b9e06c4..0e7dc87dd 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -7048,6 +7048,7 @@ conpty_term_and_job_init(
HANDLE o_theirs = NULL;
HANDLE i_ours = NULL;
HANDLE o_ours = NULL;
+ char *errmsg = NULL;

ga_init2(&ga_cmd, sizeof(char*), 20);
ga_init2(&ga_env, sizeof(char*), 20);
@@ -7149,7 +7150,10 @@ conpty_term_and_job_init(
| CREATE_SUSPENDED | CREATE_DEFAULT_ERROR_MODE,
env_wchar, cwd_wchar,
&term->tl_siex.StartupInfo, &proc_info))
+ {
+ errmsg = GetWin32Error();
goto failed;
+ }

CloseHandle(i_theirs);
CloseHandle(o_theirs);
@@ -7257,6 +7261,9 @@ failed:
if (term->tl_conpty != NULL)
pClosePseudoConsole(term->tl_conpty);
term->tl_conpty = NULL;
+ // Propagate errors that occur in CreateProcess
+ if (errmsg)
+ semsg("CreateProcess failed: %s", errmsg);
return FAIL;
}

diff --git a/src/testdir/test_terminal3.vim b/src/testdir/test_terminal3.vim
index 5049f913c..b332ff4e6 100644
--- a/src/testdir/test_terminal3.vim
+++ b/src/testdir/test_terminal3.vim
@@ -46,9 +46,11 @@ func Test_terminal_shell_option()
" the %PATH%, "term dir" succeeds unintentionally. Use dir.com instead.
try
term dir.com /b runtest.vim
- call WaitForAssert({-> assert_match('job failed', term_getline(bufnr(), 1))})
- catch /CreateProcess/
- " ignore
+ throw 'dir.com without a shell must fail, so you will never get here'
+ catch /CreateProcess failed/
+ " ignore:
+ " winpty simply fails with "CreateProcess failed".
+ " compty fails with "CreateProcess failed: {localized failure reason}".
endtry
bwipe!

diff --git a/src/version.c b/src/version.c
index 01967fcec..9ab09606f 100644
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 2013,
/**/
2012,
/**/

John

unread,
3:41 PM (5 hours ago) 3:41 PM
to vim...@googlegroups.com
After this patch, clang (v21.1.7 on Windows 11 Pro) gives these warnings:
<snip>
clang -c -I. -Iproto -DWIN32 -DWINVER=0x0A00 -D_WIN32_WINNT=0x0A00 -DHAVE_PATHDEF -DFEAT_NORMAL -DHAVE_STDINT_H -D__USE_MINGW_ANSI_STDIO -pipe -Wall -O3 -fomit-frame-pointer -fpie -fPIE -Db_lto=true -Db_lto_mode=thin -march=native -DFEAT_GUI_MSWIN -DFEAT_CLIPBOARD os_win32.c -o gobjx86-64/os_win32.o
os_win32.c:9097:16: warning: passing 'char *' to parameter of type 'char_u *' (aka 'unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
 9097 |     acp_to_enc(acp_msg, (int)acp_len, &enc_msg, &enc_len);
      |                ^~~~~~~
proto/winclip.pro:13:25: note: passing argument to parameter 'str' here
   13 | void acp_to_enc(char_u *str, int str_size, char_u **out, int *outlen);
      |                         ^
os_win32.c:9103:26: warning: passing 'char_u *' (aka 'unsigned char *') to parameter of type 'const char *' converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
 9103 |     char *pcrlf = strstr(enc_msg, "\r\n");
      |                          ^~~~~~~
E:/msys64/ucrt64/include/string.h:103:50: note: passing argument to parameter '_Str' here
  103 |   _CONST_RETURN char *__cdecl strstr(const char *_Str,const char *_SubStr);
      |                                                  ^
os_win32.c:9107:12: warning: assigning to 'char *' from 'char_u *' (aka 'unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
 9107 |     oldmsg = enc_msg;
      |            ^ ~~~~~~~
os_win32.c:9108:12: warning: returning 'char_u *' (aka 'unsigned char *') from a function with result type 'char *' converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
 9108 |     return enc_msg;
      |            ^~~~~~~
4 warnings generated.
</snip>

Cheers
John
Reply all
Reply to author
Forward
0 new messages