* isatty: make it return true in Cygwin consoles on native Windows @ 2019-03-14 22:59 Bruno Haible 2019-03-15 9:24 ` Gisle Vanem 0 siblings, 1 reply; 7+ messages in thread From: Bruno Haible @ 2019-03-14 22:59 UTC (permalink / raw) To: bug-gnulib When a native Windows program call isatty(STDOUT_FILENO) or isatty(STDIN_FILENO), it returns true for a cmd.exe console but false for a Cygwin console. This makes no sense, because the Cygwin console is just as interactive as the cmd.exe console; only the input/output encoding is different (UTF-8 vs. GetOEMCP()). This patch fixes it. 2019-03-14 Bruno Haible <bruno@clisp.org> isatty: Make it return true in Cygwin consoles on native Windows. * lib/isatty.c: Include <string.h>. (GetProcAddress): New macro. (GetNamedPipeClientProcessIdFuncType): New type. (GetNamedPipeClientProcessIdFunc): New variable. (QueryFullProcessImageNameFuncType): New type. (QueryFullProcessImageNameFunc): New variable. (initialized): New variable. (initialize): New function. (IsCygwinConsoleHandle): New function. (isatty): Invoke it. * doc/posix-functions/isatty.texi: Mention the issue. diff --git a/doc/posix-functions/isatty.texi b/doc/posix-functions/isatty.texi index 61c55da..b5e196b 100644 --- a/doc/posix-functions/isatty.texi +++ b/doc/posix-functions/isatty.texi @@ -12,6 +12,8 @@ Portability problems fixed by Gnulib: On native Windows, this function also returns true for character devices such as @file{NUL}. @item +On native Windows, this function returns false for Cygwin consoles. +@item This function crashes when invoked with invalid arguments on some platforms: MSVC 14. @end itemize diff --git a/lib/isatty.c b/lib/isatty.c index 71b32dd..085cd48 100644 --- a/lib/isatty.c +++ b/lib/isatty.c @@ -22,6 +22,7 @@ /* This replacement is enabled on native Windows. */ #include <errno.h> +#include <string.h> /* Get declarations of the Win32 API functions. */ #define WIN32_LEAN_AND_MEAN @@ -38,12 +39,90 @@ # include <io.h> #endif +/* Avoid warnings from gcc -Wcast-function-type. */ +#define GetProcAddress \ + (void *) GetProcAddress + +/* GetNamedPipeClientProcessId was introduced only in Windows Vista. */ +typedef BOOL (WINAPI * GetNamedPipeClientProcessIdFuncType) (HANDLE hPipe, + PULONG pClientProcessId); +static GetNamedPipeClientProcessIdFuncType GetNamedPipeClientProcessIdFunc = NULL; +/* QueryFullProcessImageName was introduced only in Windows Vista. */ +typedef BOOL (WINAPI * QueryFullProcessImageNameFuncType) (HANDLE hProcess, + DWORD dwFlags, + LPSTR lpExeName, + PDWORD pdwSize); +static QueryFullProcessImageNameFuncType QueryFullProcessImageNameFunc = NULL; +static BOOL initialized = FALSE; + +static void +initialize (void) +{ + HMODULE kernel32 = LoadLibrary ("kernel32.dll"); + if (kernel32 != NULL) + { + GetNamedPipeClientProcessIdFunc = + (GetNamedPipeClientProcessIdFuncType) GetProcAddress (kernel32, "GetNamedPipeClientProcessId"); + QueryFullProcessImageNameFunc = + (QueryFullProcessImageNameFuncType) GetProcAddress (kernel32, "QueryFullProcessImageNameA"); + } + initialized = TRUE; +} + static BOOL IsConsoleHandle (HANDLE h) { DWORD mode; + /* GetConsoleMode + <https://docs.microsoft.com/en-us/windows/console/getconsolemode> */ return GetConsoleMode (h, &mode) != 0; } +static BOOL IsCygwinConsoleHandle (HANDLE h) +{ + /* A handle to a Cygwin console is in fact a named pipe whose client process + and server process is <CYGWIN_INSTALL_DIR>\bin\mintty.exe. */ + BOOL result = FALSE; + ULONG processId; + + if (!initialized) + initialize (); + + /* GetNamedPipeClientProcessId + <https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-getnamedpipeclientprocessid> + It requires -D_WIN32_WINNT=_WIN32_WINNT_VISTA or higher. */ + if (GetNamedPipeClientProcessIdFunc && QueryFullProcessImageNameFunc + && GetNamedPipeClientProcessIdFunc (h, &processId)) + { + /* OpenProcess + <https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-openprocess> */ + HANDLE processHandle = + OpenProcess (PROCESS_QUERY_LIMITED_INFORMATION, FALSE, processId); + if (processHandle != NULL) + { + char buf[1024]; + DWORD bufsize = sizeof (buf); + /* The file name can be determined through + GetProcessImageFileName + <https://docs.microsoft.com/en-us/windows/desktop/api/psapi/nf-psapi-getprocessimagefilenamea> + or + QueryFullProcessImageName + <https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-queryfullprocessimagenamea> + The former returns a file name in non-standard notation (it starts + with '\Device\') and may require linking with psapi.dll. + The latter is better, but requires -D_WIN32_WINNT=_WIN32_WINNT_VISTA + or higher. */ + if (QueryFullProcessImageNameFunc (processHandle, 0, buf, &bufsize)) + { + if (strlen (buf) >= 11 + && strcmp (buf + strlen (buf) - 11, "\\mintty.exe") == 0) + result = TRUE; + } + CloseHandle (processHandle); + } + } + return result; +} + #if HAVE_MSVC_INVALID_PARAMETER_HANDLER static int _isatty_nothrow (int fd) @@ -84,6 +163,8 @@ isatty (int fd) if (IsConsoleHandle (h)) return 1; } + if (IsCygwinConsoleHandle (h)) + return 1; errno = ENOTTY; return 0; } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: isatty: make it return true in Cygwin consoles on native Windows 2019-03-14 22:59 isatty: make it return true in Cygwin consoles on native Windows Bruno Haible @ 2019-03-15 9:24 ` Gisle Vanem 2019-03-15 11:32 ` Bruno Haible 0 siblings, 1 reply; 7+ messages in thread From: Gisle Vanem @ 2019-03-15 9:24 UTC (permalink / raw) To: bug-gnulib Bruno Haible wrote: > + if (QueryFullProcessImageNameFunc (processHandle, 0, buf, &bufsize)) > + { > + if (strlen (buf) >= 11 > + && strcmp (buf + strlen (buf) - 11, "\\mintty.exe") == 0) What if the .exe has another name? The purpose of your function looks similar to what Mihail Konev did for the MinGW-w64 project: https://mingw-w64-public.narkive.com/s7kYRrtG/patch-mingw-w64-add-include-iscygtty-c Instead he looked for pipes matching: "\Device\NamedPipe\(cygwin|msys)-[a-fA-F0-9]{16}-pty[0-9]{1,4}-(from-master|to-master|to-master-cyg)" BTW. I created my own version of in his work in my Envtool program. The 'is_cygwin_tty()' function: https://github.com/gvanem/EnvTool/blob/master/src/misc.c#L3918 -- --gv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: isatty: make it return true in Cygwin consoles on native Windows 2019-03-15 9:24 ` Gisle Vanem @ 2019-03-15 11:32 ` Bruno Haible 2019-03-15 18:49 ` Gisle Vanem 0 siblings, 1 reply; 7+ messages in thread From: Bruno Haible @ 2019-03-15 11:32 UTC (permalink / raw) To: bug-gnulib; +Cc: Gisle Vanem Hi Gisle, > > + && strcmp (buf + strlen (buf) - 11, "\\mintty.exe") == 0) > > What if the .exe has another name? What other names are in common use for this program? > The purpose of your function looks similar to what Mihail Konev > did for the MinGW-w64 project: > https://mingw-w64-public.narkive.com/s7kYRrtG/patch-mingw-w64-add-include-iscygtty-c > > Instead he looked for pipes matching: > "\Device\NamedPipe\(cygwin|msys)-[a-fA-F0-9]{16}-pty[0-9]{1,4}-(from-master|to-master|to-master-cyg)" I prefer to avoid the ntdll.dll API when possible. Bruno ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: isatty: make it return true in Cygwin consoles on native Windows 2019-03-15 11:32 ` Bruno Haible @ 2019-03-15 18:49 ` Gisle Vanem 2019-03-15 19:42 ` Bruno Haible 0 siblings, 1 reply; 7+ messages in thread From: Gisle Vanem @ 2019-03-15 18:49 UTC (permalink / raw) To: bug-gnulib@gnu.org, Bruno Haible Bruno Haible wrote: >> What if the .exe has another name? > > What other names are in common use for this program? No sure. I'll need to check the MinGW-w64 list and motive. Some custom variation of it perhaps? >> The purpose of your function looks similar to what Mihail Konev >> did for the MinGW-w64 project: >> https://mingw-w64-public.narkive.com/s7kYRrtG/patch-mingw-w64-add-include-iscygtty-c >> >> Instead he looked for pipes matching: >> "\Device\NamedPipe\(cygwin|msys)-[a-fA-F0-9]{16}-pty[0-9]{1,4}-(from-master|to-master|to-master-cyg)" > > I prefer to avoid the ntdll.dll API when possible. Okay, what's wrong with that? -- --gv ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: isatty: make it return true in Cygwin consoles on native Windows 2019-03-15 18:49 ` Gisle Vanem @ 2019-03-15 19:42 ` Bruno Haible 2019-03-15 20:16 ` LRN 0 siblings, 1 reply; 7+ messages in thread From: Bruno Haible @ 2019-03-15 19:42 UTC (permalink / raw) To: Gisle Vanem; +Cc: bug-gnulib@gnu.org Gisle Vanem asked: > > I prefer to avoid the ntdll.dll API when possible. > > Okay, what's wrong with that? 1) It's a violation of abstraction. Diagram: Programs | Windows API (kernel32.dll etc.) | NT API (ntdll.dll) | NT kernel Programs should use only topmost API layer. When you use a mix between two or more layers, i.e. when you circumvent the topmost API layer, in my experience this causes big problems in the long run. 2) The code you pointed to uses the function NtQueryObject. However, the Microsoft documentation <https://docs.microsoft.com/en-us/windows/desktop/api/winternl/nf-winternl-ntqueryobject> states "This function may be changed or removed from Windows without further notice." 3) Probably code will run better on ReactOS or WINE if they don't use the lower layers. Bruno ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: isatty: make it return true in Cygwin consoles on native Windows 2019-03-15 19:42 ` Bruno Haible @ 2019-03-15 20:16 ` LRN 2019-03-15 22:24 ` Bruno Haible 0 siblings, 1 reply; 7+ messages in thread From: LRN @ 2019-03-15 20:16 UTC (permalink / raw) To: bug-gnulib [-- Attachment #1.1: Type: text/plain, Size: 1301 bytes --] On 15.03.2019 22:42, Bruno Haible wrote: > Gisle Vanem asked: >>> I prefer to avoid the ntdll.dll API when possible. >> >> Okay, what's wrong with that? > > 1) It's a violation of abstraction. > > 2) The code you pointed to uses the function NtQueryObject. However, the > Microsoft documentation > <https://docs.microsoft.com/en-us/windows/desktop/api/winternl/nf-winternl-ntqueryobject> > states "This function may be changed or removed from Windows without > further notice." > > 3) Probably code will run better on ReactOS or WINE if they don't use the > lower layers. > Advanced functionality sometimes requires the use of kernel API in cases where Microsoft decided not to expose some NT kernel functions to applications in Win32 API. These situations happen from time to time when dealing with portability. You can lessen the impact by configure-time-testing the kernel APIs to ensure that they are available and behave as expected. In some sense this is kind of like using very Linux-specific functions. If the functions vanish, you'll notice. Also ReactOS and WINE won't use that code, if they have no functions and thus fail the configure-tests. Either way, some things cannot be implemented in a clear and/or performant way without these. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: isatty: make it return true in Cygwin consoles on native Windows 2019-03-15 20:16 ` LRN @ 2019-03-15 22:24 ` Bruno Haible 0 siblings, 0 replies; 7+ messages in thread From: Bruno Haible @ 2019-03-15 22:24 UTC (permalink / raw) To: bug-gnulib; +Cc: LRN LRN wrote: > Either way, some things cannot be implemented in a clear and/or performant way > without these. [ntdll] Sure, I don't dispute this. But when it's possible - like in this case with isatty() - I prefer to rely only on the Windows API. Bruno ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-15 22:25 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-14 22:59 isatty: make it return true in Cygwin consoles on native Windows Bruno Haible 2019-03-15 9:24 ` Gisle Vanem 2019-03-15 11:32 ` Bruno Haible 2019-03-15 18:49 ` Gisle Vanem 2019-03-15 19:42 ` Bruno Haible 2019-03-15 20:16 ` LRN 2019-03-15 22:24 ` Bruno Haible
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).