Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(556)

Issue 13636003: Fix a number of issues with determining the type of stdio (Closed)

Created:
7 years, 8 months ago by Søren Gjesse
Modified:
7 years, 8 months ago
Reviewers:
Mads Ager (google)
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Fix a number of issues with determining the type of stdio * Fix checked mode exception when _getSocketType returns an OSError * Treat any character device as terminal (this includes /dev/null) * Fix detecting of file redirection af file * Don't turn an unkonwn device type into an OSError (wth value for an unknown was -1) Expanded an existing test to catch some of this. R=ager@google.com BUG=https://code.google.com/p/dart/issues/detail?id=9637 Committed: https://code.google.com/p/dart/source/detail?r=20960

Patch Set 1 #

Patch Set 2 : Avoid sharing of Dart scripts between different procsess tests #

Patch Set 3 : Add fix for Mac OS as well #

Total comments: 4

Patch Set 4 : Addressed review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -14 lines) Patch
M runtime/bin/file.h View 2 chunks +2 lines, -2 lines 0 comments Download
M runtime/bin/socket_linux.cc View 1 chunk +1 line, -1 line 0 comments Download
M runtime/bin/socket_macos.cc View 1 2 1 chunk +1 line, -1 line 0 comments Download
M runtime/bin/stdio_patch.dart View 1 chunk +5 lines, -1 line 0 comments Download
M sdk/lib/io/stdio.dart View 3 chunks +4 lines, -3 lines 0 comments Download
A + tests/standalone/io/dart_std_io_pipe_script.dart View 1 1 chunk +12 lines, -0 lines 0 comments Download
M tests/standalone/io/dart_std_io_pipe_test.dart View 1 2 3 2 chunks +9 lines, -3 lines 0 comments Download
M tests/standalone/io/dart_std_io_pipe_test.sh View 1 chunk +7 lines, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Søren Gjesse
On Windows all character devices was already reported as terminal (which is the case for ...
7 years, 8 months ago (2013-04-04 13:22:35 UTC) #1
Mads Ager (google)
LGTM https://codereview.chromium.org/13636003/diff/4001/runtime/bin/file_linux.cc File runtime/bin/file_linux.cc (right): https://codereview.chromium.org/13636003/diff/4001/runtime/bin/file_linux.cc#newcode1 runtime/bin/file_linux.cc:1: // Copyright (c) 2013, the Dart project authors. ...
7 years, 8 months ago (2013-04-05 05:34:28 UTC) #2
Søren Gjesse
Committed patchset #4 manually as r20960 (presubmit successful).
7 years, 8 months ago (2013-04-05 11:13:38 UTC) #3
Søren Gjesse
7 years, 8 months ago (2013-04-09 11:35:20 UTC) #4
Message was sent while issue was closed.
https://codereview.chromium.org/13636003/diff/4001/runtime/bin/file_linux.cc
File runtime/bin/file_linux.cc (right):

https://codereview.chromium.org/13636003/diff/4001/runtime/bin/file_linux.cc#...
runtime/bin/file_linux.cc:1: // Copyright (c) 2013, the Dart project authors. 
Please see the AUTHORS file
On 2013/04/05 05:34:28, Mads Ager wrote:
> Revert this file? Maybe disable the editor magic that does these updates for
> you? 

Done.

https://codereview.chromium.org/13636003/diff/4001/tests/standalone/io/dart_s...
File tests/standalone/io/dart_std_io_pipe_test.dart (right):

https://codereview.chromium.org/13636003/diff/4001/tests/standalone/io/dart_s...
tests/standalone/io/dart_std_io_pipe_test.dart:115: scriptFile = new
File("../tests/standalone/io/dart_std_io_pipe_script.dart");
On 2013/04/05 05:34:28, Mads Ager wrote:
> long line

Done.

Powered by Google App Engine
This is Rietveld 408576698