Make std* blocking file-descriptors.

Make std* blocking file-descriptors.

This will most likely be a performance regression when piping, that
we'll have to look into in the future (copying data through
    message-passing).

BUG=
R=sgjesse@google.com

Committed: https://code.google.com/p/dart/source/detail?r=32715

[Anders Johnsen, 2014-02-07 10:16:35]

*linux only* - let me know what you think.

[Søren Gjesse, 2014-02-07 11:05:10]

This actually cleaned up stuff.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc
File runtime/bin/file_linux.cc (right):

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:48: if (handle_->fd() == STDOUT_FILENO) {
Why is stderr not handled here as well?

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:51: ASSERT(null_fd >= 0);
So this will still affect "printf debugging".

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:52: VOID_TEMP_FAILURE_RETRY(dup2(null_fd,
STDOUT_FILENO));
Use handle_->fd() instead of STDOUT_FILENO here.

https://codereview.chromium.org/154273003/diff/1/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/154273003/diff/1/sdk/lib/io/stdio.dart#newcod...
sdk/lib/io/stdio.dart:277: switch (_getStdioHandleType(object == stdout ? 1 :
2)) {
Use the STDOUT_FILENO and STDERR_FILENO constants here instead of 1 and 2.

[Ivan Posva, 2014-02-11 04:08:36]

-Ivan

https://codereview.chromium.org/154273003/diff/1/runtime/bin/builtin_natives.cc
File runtime/bin/builtin_natives.cc (right):

https://codereview.chromium.org/154273003/diff/1/runtime/bin/builtin_natives....
runtime/bin/builtin_natives.cc:124: fflush(stdout);
If you are using fflush here it would be consistent to use fprintf(stdout,
above.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc
File runtime/bin/file_linux.cc (right):

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:52: VOID_TEMP_FAILURE_RETRY(dup2(null_fd,
STDOUT_FILENO));
I am wondering whether we should have duped the stdout and stderr fds at the
beginning when opening them from dart:io.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/stdio_patch.dart
File runtime/bin/stdio_patch.dart (right):

https://codereview.chromium.org/154273003/diff/1/runtime/bin/stdio_patch.dart...
runtime/bin/stdio_patch.dart:28: switch (_getStdioHandleType(fd)) {
Do we now really care what kind of file type stdout or stderr are?

[Anders Johnsen, 2014-02-14 08:43:28]

Starting to port this.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/builtin_natives.cc
File runtime/bin/builtin_natives.cc (right):

https://codereview.chromium.org/154273003/diff/1/runtime/bin/builtin_natives....
runtime/bin/builtin_natives.cc:124: fflush(stdout);
On 2014/02/11 04:08:37, Ivan Posva wrote:
> If you are using fflush here it would be consistent to use fprintf(stdout,
> above.

Done.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc
File runtime/bin/file_linux.cc (right):

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:48: if (handle_->fd() == STDOUT_FILENO) {
On 2014/02/07 11:05:11, Søren Gjesse wrote:
> Why is stderr not handled here as well?

It's only to work around print printing to file/socket, that's reusing the
stdout fd.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:51: ASSERT(null_fd >= 0);
On 2014/02/07 11:05:11, Søren Gjesse wrote:
> So this will still affect "printf debugging".

Yes, intentionally. :)

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:52: VOID_TEMP_FAILURE_RETRY(dup2(null_fd,
STDOUT_FILENO));
On 2014/02/07 11:05:11, Søren Gjesse wrote:
> Use handle_->fd() instead of STDOUT_FILENO here.

Done.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/file_linux.cc#ne...
runtime/bin/file_linux.cc:52: VOID_TEMP_FAILURE_RETRY(dup2(null_fd,
STDOUT_FILENO));
On 2014/02/11 04:08:37, Ivan Posva wrote:
> I am wondering whether we should have duped the stdout and stderr fds at the
> beginning when opening them from dart:io.

That would still have given us this scenario where stdout.close and print can do
bad things. Note that the fact that we can close stdout is a feature.

https://codereview.chromium.org/154273003/diff/1/runtime/bin/stdio_patch.dart
File runtime/bin/stdio_patch.dart (right):

https://codereview.chromium.org/154273003/diff/1/runtime/bin/stdio_patch.dart...
runtime/bin/stdio_patch.dart:28: switch (_getStdioHandleType(fd)) {
On 2014/02/11 04:08:37, Ivan Posva wrote:
> Do we now really care what kind of file type stdout or stderr are?

One should consider stdout and not stdin when deciding if you want to use
colors.

https://codereview.chromium.org/154273003/diff/1/sdk/lib/io/stdio.dart
File sdk/lib/io/stdio.dart (right):

https://codereview.chromium.org/154273003/diff/1/sdk/lib/io/stdio.dart#newcod...
sdk/lib/io/stdio.dart:277: switch (_getStdioHandleType(object == stdout ? 1 :
2)) {
On 2014/02/07 11:05:11, Søren Gjesse wrote:
> Use the STDOUT_FILENO and STDERR_FILENO constants here instead of 1 and 2.

They are not available in Dart.

[Anders Johnsen, 2014-02-14 11:01:27]

PTAL, now working for all platforms.

[Søren Gjesse, 2014-02-14 12:16:04]

LGTM!

[Anders Johnsen, 2014-02-17 10:06:58]

Committed patchset #6 manually as r32715 (presubmit successful).