Hi Vincent, in isStdioDescriptorATTY at line 898 there is no guard to check if pGetFileInformationByHandleEx has already been fetched. The code needs to be something like

static pfnGetFileInformationByHandleEx pGetFileInformationByHandleEx = NULL;

if (! pGetFileInformationByHandleEx) { pGetFileInformationByHandleEx = (pfnGetFileInformationByHandleEx) GetProcAddress(GetModuleHandle(TEXT("kernel32.dll")), "GetFileInformationByHandleEx"); if (pGetFileInformationByHandleEx == NULL) return -2; }

Thanks Eliot for the piece of advice! I am not an expert in C yet ;)

I made the changes but I was wondering why it is needed to set a guard: the line before just set the pGetFileInformationByHandleEx to NULL.

Hi Vincent,

Just based on a quick visual inspection of the code:

- It looks like on Unix sqStdioDescriptorIsATTY() checks stdin, while on Windows it checks stdout, which is a bit inconsistent and they can be redirected independently anyway. Why not pass an argument allowing the user to specify which file to check (0, 1, 2)? - We'll want to modify sqFileStdioHandlesInto() in sqWin32FilePrims.c to use the new check (instead of GetConsoleMode()). - Actually, since the result is always stored in the SQFile structure for the stdio streams, the primitive could just return the cached value rather than determining it every time. - sqFileAtEnd() on Windows just assumes that stdin is never at EOF if it is a terminal. I don't know how EOF is signalled in a terminal on Windows (Ctrl-D?), but we could also improve this (although maybe this can be a separate PR).

Thanks! Alistair

@VincentBlondeau pushed 2 commits.

cf60cec Add fd as parameter for sqStdioDescriptorIsATTY 88546ed get rid of getConsoleMode in sqFileStdioHandlesInto + fix bug on powershell

Reopened #254.

The exposed primitive sqIsFileDescriptorATTY should not receive an HANDLE because one needs to call sqIsFileDescriptorATTY to know if an handle is available. But I can change sqWin32FilePrims.c:isFileDescriptorATTY to take an HANDLE as argument instead of a String

@VincentBlondeau pushed 1 commit.

702a540 rename isFileDescriptorATTY to isFileHandleATTY

@VincentBlondeau pushed 1 commit.

f9e79db Use the right functions to deal with WINAPI HANDLEs + refactoring

Hi Vincent,

This still isn't working because sqFileStdioHandlesIntoFile_WithHandle_IsWritable() doesn't take SQFile by reference (arrays are just pointers to the first element in C, which is why the array of SQFile structures doesn't need to be passed by reference explicitly):

``` sqFileStdioHandlesIntoFile_WithHandle_IsWritable(SQFile *file, HANDLE handle, int isWritable) { file->sessionID = thisSession; file->file = handle; file->writable = isWritable; file->lastOp = 0; /* unused on win32 */ file->isStdioStream = isFileHandleATTY(handle); AddHandleToTable(win32Files, handle); }

sqFileStdioHandlesInto(SQFile files[3]) { sqFileStdioHandlesIntoFile_WithHandle_IsWritable(&files[0], GetStdHandle(STD_INPUT_HANDLE), false); sqFileStdioHandlesIntoFile_WithHandle_IsWritable(&files[1], GetStdHandle(STD_OUTPUT_HANDLE), true); sqFileStdioHandlesIntoFile_WithHandle_IsWritable(&files[2], GetStdHandle(STD_ERROR_HANDLE), true);

return 7; } ```

I think that gets the basic changes you wanted done. But it still leaves stdio in cygwin terminals broken. After a bit more reading it looks like cygwin terminals and Windows ReadConsole() and WriteConsole() functions are fundamentally incompatible.

I don't understand what ReadConsole() and WriteConsole() provide in terms of benefits over FILE* streams, so I can't really comment on the cost of replacing them.

Eliot, what are your thoughts about moving the stdio functionlity to use FILE* streams instead of HANDLE streams?

I think that would allow the VM to use fread(), feof(), and fwrite() to read and write stdio to terminals (cygwin), consoles (windows), pipes and regular files.

Another option may be to leave the existing console functionality and use the connectToFileDescriptor() routine to separately open the stdio streams as FILE* streams. The image can then decide how it wants to interact with stdio.

(we should probably move this conversation to vm-dev and out of this particular PR)

Thanks, Alistair

@VincentBlondeau pushed 1 commit.

2a95124 An array of structs have to be passed as reference...

Hi Vincent & Eliot,

What do you mean by broken? Do you have an example I can reproduce?

Sure - if you start PharoConsole.exe from within a cygwin (mintty) terminal and remove the #isWin32 block from Stdio class>>standardIOStreamNamed:forWrite: so that you are actually using stdout, and then attempt to write something to stdout you'll get a primitive failure. The same sequence works from a dos box (cmd.exe).

That's because the read and write primitives rely on the Windows ReadConsole() and WriteConsole() functions, which only work in a dos box, not in mintty.

When we started these modifications I assumed that it would be possible to have I/O that worked in both cygwin (mintty) and a dos box (cmd.exe). However on further reading it looks like MingW programs (which use either Windows functions such as ReadConsole() and WriteConsole() or the MS posix implementation) will only work properly in a dos box. If you want proper mintty support you need to use the cygwin implementation of the posix functions.

As an example, attempting to use the MingW/MS fread() function and signal EOF:

- In a dos box, typing Control-Z correctly signals EOF. - In a cygwin terminal (mintty), Control-D is never passed to the program, so EOF is never true. (If you then type Control-C to terminate the program, sometimes the terminal then consumes the earlier Control-D and logs you out :-().

Someone please explain how I am wrong and how to get stdio working on both dos boxes and cygwin terminals...

Thanks, Alistair

Hi Vincent & Eliot,

Ugh. I didn't realize one couldn't use the same code in both :-(. I guess we have to test to find out what the context is and use dread in one and ReadConsole in the other.

I think it is worse than that. If I use the following test code in a cygwin terminal:

``` #include <stdio.h> #include <stdlib.h>

int main() { char buf[1024]; char *bufp; int cread; int count = 0;

bufp = &buf[0]; do { cread = fread(bufp, 1, 1, stdin); count += cread; bufp += cread; if (count > 16) break; } while (cread > 0); buf[count] = 0;

if (! feof(stdin)) fprintf(stderr, "Error, not at end of file\n");

printf("--\n"); printf("%s\n", buf); printf("--\n"); printf("Read %d characters\n", count); exit(0); } ```

And compile it with mingw:

``` $ i686-w64-mingw32-gcc -m32 consolestdio.c ```

It won't recognise EOF (Control-D).

``` $ ./a.exe hello world <Ctrl-D> asdfasdfasdfasdfasdf -- hello world

asdf -- Read 17 characters Error, not at end of file ```

If I compile it with gcc:

``` $ gcc -m32 consolestdio.c ```

It works as expected:

``` $ ./a.exe hello world <Ctrl-D> -- hello world

-- Read 12 characters ```

So the best option I see for supporting both cygwin terminals and dos boxes is to make our own dll that wraps around the cygwin1.dll functions and to load it as required. Although I'm a bit concerned that I haven't found any mention of doing this elsewhere (and it's a lot of work for one specific use case).

But I don't get it. IIRC, it used to be the case that ReadConsole/WriteConsole worked in both.

I'd still like to be proved wrong...

Thanks, Alistair

Hi Ben,

Try Ctrl-Z "In a non-Cygwin Windows program, Ctrl-Z on input triggers an end-of-file condition."

I had tried this, and since it's bash it just stopped the program, as expected.

I'd also looked at the page you referenced, but on reading it again thought I'd try unsetting the susp key:

``` stty susp '' ```

And sure enough, running the mingw version works this time:

``` $ ./mingw.exe asdf<Enter> <Ctrl-Z><Enter> -- asdf

-- Read 5 characters ```

While it is a workaround, I'm not sure that asking users to modify the terminal settings and remember to use Ctrl-Z instead of Ctrl-D for Pharo in bash is going to be practical.

What do you think?

Just for LOLs, the following does work:

``` $ cat | vm/PharoConsole Pharo7.0.image eval StdioStreamTest manualStdinTest | tee /dev/null asdf<Enter> <Ctrl-D> ```

It doesn't work in headless mode, I haven't figured out why yet.

Cheers, Alistair

So this is real goodness. Can we not address the issues and get this in? Right now as I understand it we know how to discover both a Cygwin console and a Windows Console, but the two require separate approaches. Can we not add both of them? As far as the end-of-file issue dgoes I would say that is not a deal breaker. If I were implementing a REPL I would want EOF to cause the system to exit cleanly but I would also live with having to issue a proper quit command.

Hi Alistair,

I agree we shouldn't include cygwin1.dll. But the existence of cygwin1.dll is proof that the relevant code can be implemented above Win32. I do know that one can read from cygwin terminals; I was doing that regularly with the old VM. So the issue is simply identifying a cygwin console as a tty. We can put reading/writing from/to it to one side and first focus on identification. I'm happy for you to extend primitiveIsFileDescriptorATTY as you suggest. What I think we need, though, is to be able to identify either a Windows console or a cygwin console independently of cygwin1.dll (even if we have to copy some liberally licensed code from it).

Sounds good! The only thing is I can check the result only under windows. So, I don't know which values to return for Linux if the terminal is not a tty. Should I return 2 or 3? And how do I know that is a file or a pipe?

Hi Vincent,

I assumed that for Windows you could already distinguish between pipes and normal files since the pty check uses the pipe name as part of its check. If not...

The fstat() st_mode field has S_IFIFO which indicates a first-in first-out (pipe) file.

Some of the documentation I saw said that S_IFIFO is for named pipes, I don't know what it will return for stdin being a pipe.

fstat() takes a file descriptor, so you'll need to convert a HANDLE to a fd.

If you can get Windows working I'm happy to take care of the Unix side.

Cheers, Alistair

@VincentBlondeau pushed 1 commit.

0ec7f5d Renaming of the primitive + change return value

Hi Vincent,

In cygwin you can test pipes with:

``` $ cat | PharoConsole Pharo7.0.image | tee /dev/null ```

It's also a hacky way to get terminal i/o working in a cygwin terminal.

In a windows console:

``` echo "hello world" | PharoConsole Pharo7.0.image ```

Should allow you to test stdin at least (untested).

In both cygwin terminals and windows consoles file i/o can be tested with:

``` PharoConsole Pharo7.0.image < a.txt > b.txt ```

HTH, Alistair

@VincentBlondeau pushed 1 commit.

48101cd Return Error for RiskOS

@VincentBlondeau pushed 1 commit.

82d6abc Use methodReturn***: instead of pop: + push***: in FilePlugin

Merged #254.