Skip to content

Conversation

sfackler
Copy link
Member

Closes #25977

The various stdfoo_raw methods in std::io now return io::Results,
since they may not exist on Windows. They will always return Ok on
Unix-like platforms.

[breaking-change]

@rust-highfive
Copy link
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@sfackler
Copy link
Member Author

r? @alexcrichton

cc @retep998

enum MaybeRead<R> {
Real(R),
Empty,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this could just be:

enum Maybe<T> {
    Real(T),
    Empty,
}

impl<W: Write> Write for Maybe<W> { ... }
impl<R: Read> Read for Maybe<W> { ... }

@alexcrichton
Copy link
Member

Thanks @sfackler! A few notes:

  • I believe the Unix side should start handling EBADF in the read/write implementations of the stdio handles to mirror the same behavior as Windows (e.g. a write on a nonexistent stream is just a sink).
  • Could you add a test which exercises this behavior? I think it'll have to do platform-specific things to close the stdio descriptor, but that shouldn't be too bad.

@sfackler
Copy link
Member Author

Updated

pub fn new() -> Stdout {
Stdout(get(c::STD_OUTPUT_HANDLE).unwrap())
pub fn new() -> io::Result<Stdout> {
Stdout(try!(get(c::STD_OUTPUT_HANDLE)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a surrounding Ok (above as well), perhaps jut a get(...).map(|handle| ...) ?

@alexcrichton
Copy link
Member

Just a few minor nits, but otherwise r=me, thanks @sfackler!

@sfackler
Copy link
Member Author

@bors r=alexcrichton 517d3bd

@bors
Copy link
Collaborator

bors commented Jun 12, 2015

⌛ Testing commit 517d3bd with merge bdde81d...

@bors
Copy link
Collaborator

bors commented Jun 12, 2015

💔 Test failed - auto-win-gnu-64-opt

#[cfg(windows)]
fn close_stdout() {
pub const STD_OUTPUT_HANDLE: libc::DWORD = -11i32 as libc::DWORD;
unsafe { libc::CloseHandle(STD_OUTPUT_HANDLE); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. STD_OUTPUT_HANDLE isn't a HANDLE so you need to GetStdHandle to get the actual HANDLE to CloseHandle.
  2. Write a separate test which does SetStdHandle(STD_OUTPUT_HANDLE, NULL) so it can test println!'s ability to replace stdout with a sink, rather than just stdout's ability to ignore errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed.

@sfackler sfackler force-pushed the stdout-panic branch 4 times, most recently from 155dd8f to 7e4ae36 Compare June 15, 2015 03:05
Closes rust-lang#25977

The various `stdfoo_raw` methods in std::io now return `io::Result`s,
since they may not exist on Windows. They will always return `Ok` on
Unix-like platforms.

[breaking-change]
@sfackler
Copy link
Member Author

@bors r=alexcrichton a7bbd7d

@bors
Copy link
Collaborator

bors commented Jun 15, 2015

⌛ Testing commit a7bbd7d with merge 57cd1c4...

@bors
Copy link
Collaborator

bors commented Jun 15, 2015

💔 Test failed - auto-linux-32-opt

@alexcrichton
Copy link
Member

@bors: retry

On Sun, Jun 14, 2015 at 10:11 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-linux-32-opt
http://buildbot.rust-lang.org/builders/auto-linux-32-opt/builds/5364


Reply to this email directly or view it on GitHub
#26168 (comment).

@bors
Copy link
Collaborator

bors commented Jun 15, 2015

⌛ Testing commit a7bbd7d with merge 7517ecf...

bors added a commit that referenced this pull request Jun 15, 2015
Closes #25977

The various `stdfoo_raw` methods in std::io now return `io::Result`s,
since they may not exist on Windows. They will always return `Ok` on
Unix-like platforms.

[breaking-change]
@bors bors merged commit a7bbd7d into rust-lang:master Jun 15, 2015
@brson brson added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Jun 23, 2015
@brson
Copy link
Contributor

brson commented Jun 23, 2015

I tagged this as stable-regression since it changes the behavior of stdio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants