Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

io safety: update Unix explanation to use Arc #124412

Merged
merged 1 commit into from
May 3, 2024
Merged

io safety: update Unix explanation to use Arc #124412

merged 1 commit into from
May 3, 2024

Conversation

RalfJung
Copy link
Member

Fixes #124384

Cc @jsgf

@rustbot
Copy link
Collaborator

rustbot commented Apr 26, 2024

r? @Amanieu

rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 26, 2024
@RalfJung RalfJung changed the title io safety: update Unix explanation io safety: update Unix explanation to use Arc Apr 26, 2024
jsgf
jsgf reviewed Apr 26, 2024
View reviewed changes
library/std/src/os/unix/io/mod.rs Outdated
@@ -23,22 +23,33 @@
//! either by adding `unsafe` to APIs that dereference `RawFd` values, or by
//! using to `BorrowedFd` or `OwnedFd` instead.
//!
//! The use of `Arc` for borrowed/owned file descriptors may be surprising.
//! Unix file descriptors are mere references to internal *file descriptions*,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't say I'm a huge fan of the term "file description" (it's too similar to "file descriptor"), but I guess we're stuck with it if that's what Posix uses.

Copy link
Member

Choose a reason for hiding this comment

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

The full name is open file description which might be slightly less confusing? Maybe saying "kernel descriptor" or "kernel file descriptor" would help clarify. But sticking with the nomenclature used elsewhere does make sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added the "open".

@jsgf
Copy link
Contributor

jsgf commented Apr 26, 2024

Yep, looks good to me.

@Amanieu
Copy link
Member

Amanieu commented May 2, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented May 2, 2024

📌 Commit 6b6bc98 has been approved by Amanieu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 2, 2024
@matthiaskrgr
Rollup merge of rust-lang#124412 - RalfJung:io-safety, r=Amanieu
198852b 
io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc `@jsgf`
@matthiaskrgr matthiaskrgr mentioned this pull request May 2, 2024
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 2, 2024
@matthiaskrgr
Rollup merge of rust-lang#124412 - RalfJung:io-safety, r=Amanieu
7ab9346 
io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc ``@jsgf``
@matthiaskrgr matthiaskrgr mentioned this pull request May 2, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2024
@bors
Auto merge of rust-lang#124633 - matthiaskrgr:rollup-x4kyjp2, r=matth…
cc02b73 
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#124345 (Enable `--check-cfg` by default in UI tests)
 - rust-lang#124412 (io safety: update Unix explanation to use `Arc`)
 - rust-lang#124441 (String.truncate comment microfix (greater or equal))
 - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature)

Failed merges:

 - rust-lang#124607 (`rustc_expand` cleanups)
 - rust-lang#124613 (Allow fmt to run on rmake.rs test files)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2024
@bors
Auto merge of rust-lang#124633 - matthiaskrgr:rollup-x4kyjp2, r=matth…
be54559 
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#124345 (Enable `--check-cfg` by default in UI tests)
 - rust-lang#124412 (io safety: update Unix explanation to use `Arc`)
 - rust-lang#124441 (String.truncate comment microfix (greater or equal))
 - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature)

Failed merges:

 - rust-lang#124607 (`rustc_expand` cleanups)
 - rust-lang#124613 (Allow fmt to run on rmake.rs test files)

r? `@ghost`
`@rustbot` modify labels: rollup
fmease added a commit to fmease/rust that referenced this pull request May 3, 2024
@fmease
Rollup merge of rust-lang#124412 - RalfJung:io-safety, r=Amanieu
cd38eae 
io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc ``@jsgf``
@fmease fmease mentioned this pull request May 3, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
@bors
Auto merge of rust-lang#124643 - fmease:rollup-esmkgxo, r=fmease
1d64561 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#124412 (io safety: update Unix explanation to use `Arc`)
 - rust-lang#124441 (String.truncate comment microfix (greater or equal))
 - rust-lang#124594 (run-make-support: preserve tooks.mk behavior for EXTRACXXFLAGS)
 - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature)
 - rust-lang#124607 (`rustc_expand` cleanups)
 - rust-lang#124609 (variable-precision float operations can differ depending on optimization levels)
 - rust-lang#124610 (Tweak `consts_may_unify`)
 - rust-lang#124612 (Add support for inputing via stdin with run-make-support)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request May 3, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
@bors
Auto merge of rust-lang#124646 - matthiaskrgr:rollup-crlsvg5, r=matth…
561b5de 
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#123480 (deref patterns: impl `DerefPure` for more std types)
 - rust-lang#124412 (io safety: update Unix explanation to use `Arc`)
 - rust-lang#124441 (String.truncate comment microfix (greater or equal))
 - rust-lang#124594 (run-make-support: preserve tooks.mk behavior for EXTRACXXFLAGS)
 - rust-lang#124604 (library/std: Remove unused `gimli-symbolize` feature)
 - rust-lang#124607 (`rustc_expand` cleanups)
 - rust-lang#124609 (variable-precision float operations can differ depending on optimization levels)
 - rust-lang#124610 (Tweak `consts_may_unify`)
 - rust-lang#124626 (const_eval_select: add tracking issue)
 - rust-lang#124637 (AST pretty: Use `builtin_syntax` for type ascription)

Failed merges:

 - rust-lang#124638 (Move some tests from `rustc_expand` to `rustc_parse`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9ab5cfd into rust-lang:master May 3, 2024
10 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2024
@rust-timer
Unrolled build for rust-lang#124412
863333e 
Rollup merge of rust-lang#124412 - RalfJung:io-safety, r=Amanieu

io safety: update Unix explanation to use `Arc`

Fixes rust-lang#124384

Cc ```@jsgf```
@RalfJung RalfJung deleted the io-safety branch May 3, 2024 13:29
@workingjubilee workingjubilee mentioned this pull request May 3, 2024
Closed
@RalfJung
Copy link
Member Author

RalfJung commented May 3, 2024

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChrisDenton ChrisDenton ChrisDenton left review comments

@jsgf jsgf jsgf left review comments

Assignees

@Amanieu Amanieu

Labels
O-unix Operating system: Unix-like S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.80.0
Development

Successfully merging this pull request may close these issues.

Module docs for std::os::unix::io inconsistent with I/O Safety discussion in std::io
6 participants
@RalfJung @rustbot @jsgf @Amanieu @bors @ChrisDenton