fix typo in documentation for std::fs::Permissions #109263

squell · 2023-03-17T13:37:11Z

Please check and re-check this PR carefully to see if I got this right.

But by my logic, if the read_only function returns true, I would not expect be able to write to the file (it being read only); so this text is meant to clarify that read_only being false doesn't mean you can actually write to the file, just that "in general" someone is able to.

rustbot · 2023-03-17T13:37:19Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thomcc (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

rustbot · 2023-03-17T13:37:22Z

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

Stabilizing library features
Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
Changing public documentation in ways that create new stability guarantees
Changing observable runtime behavior of library APIs

thomcc · 2023-03-30T22:40:31Z

I see your confusion, the issue probably stems from the fact that this is "readonly" rather than "readable" or something.

thomcc

I can see what you mean, but also why it was written the way it was. I honestly think it's a confusing way to phrase it, so probably it's worth rewording the sentence instead.

thomcc · 2023-05-05T11:34:23Z

library/std/src/fs.rs

@@ -1406,7 +1406,7 @@ impl Permissions {
    /// On Unix-based platforms this checks if *any* of the owner, group or others
    /// write permission bits are set. It does not check if the current
    /// user is in the file's assigned group. It also does not check ACLs.
-    /// Therefore even if this returns true you may not be able to write to the
+    /// Therefore even if this returns false you may not be able to write to the


Perhaps this should be

Therefore even if this returns true you may not be able to read to the file

Or even

Therefore you may not be able to rely on the return value of this function to predict the success of reads or writes on the file.

I think the second approach is preferable; the first one is better than the original (since it is accurate), but changes the text from talking about writes to reads (while the focus of the read_only function is checking the write permissions).

I.e. any text should allow the reader to make the inference that:

if it returns true doesn't say anything about being actually able to read (all the UNIX read permissions could be unset)

if it returns false that doesn't mean you can write to the file

thomcc · 2023-05-05T11:35:23Z

@rustbot author

Dylan-DPC · 2023-08-07T19:14:00Z

@squell any updates on this? thanks

squell · 2023-08-29T09:25:09Z

Reworded in the style suggested by @thomcc; but a bit stronger worded (if someone "may not be able to rely", that just means they can't rely); and phrased "the success of reads of writes" in a more active voice.

Noratrieb · 2024-01-22T15:12:57Z

library/std/src/fs.rs

+    /// to predict whether attempts to read or write the file will actually succeed.
+    /// The [`PermissionsExt`] trait gives direct access to the permission bits but
+    /// also does not read ACLs. If you need to accurately know whether or not a file
+    /// is writable use the `access()` function from libc.


Suggested change

/// is writable use the `access()` function from libc.

/// is writable use the `access()` function from libc.

I think it's work mentioning that using access can lead to time-of-check-time-of-use problems where the permissions get changed between the call to access and the actual access, and therefore the return value of access also cannot be relied on for some things. The only way to figure out whether you can open a file is to open it.

Perhaps the final sentence should simply be removed lest the documentation for this function turn into a tutorial about interacting with a file system.

That sounds good.

I agree with removing that line.

thomcc · 2024-02-01T22:53:09Z

I'm going to be away for a few months, so I'm rerolling my PRs so that folks don't have to wait for me. Sorry/thanks.

r? libs

joshtriplett · 2024-02-21T17:48:18Z

(waiting for an update that deletes the last sentence)

cuviper · 2024-03-02T00:51:10Z

@bors r+ rollup

bors · 2024-03-02T00:51:12Z

📌 Commit b353765 has been approved by cuviper

It is now in the queue for this repository.

fix typo in documentation for std::fs::Permissions Please check and re-check this PR carefully to see if I got this right. But by my logic, if the `read_only` function returns `true`, I would not expect be able to write to the file (it being read only); so this text is meant to clarify that `read_only` being `false` doesn't mean *you* can actually write to the file, just that "in general" someone is able to.

…iaskrgr Rollup of 10 pull requests Successful merges: - rust-lang#109263 (fix typo in documentation for std::fs::Permissions) - rust-lang#120684 (Update E0716.md for clarity) - rust-lang#121739 (Display short types for unimplemented trait) - rust-lang#121749 (Don't lint on executable crates with `non_snake_case` names) - rust-lang#121758 (Move thread local implementation to `sys`) - rust-lang#121815 (Move `gather_comments`.) - rust-lang#121835 (Move `HandleStore` into `server.rs`.) - rust-lang#121847 (Remove hidden use of Global) - rust-lang#121861 (Use the guaranteed precision of a couple of float functions in docs) - rust-lang#121875 ( Account for unmet T: !Copy in E0277 message) r? `@ghost` `@rustbot` modify labels: rollup

fix typo in documentation for std::fs::Permissions Please check and re-check this PR carefully to see if I got this right. But by my logic, if the `read_only` function returns `true`, I would not expect be able to write to the file (it being read only); so this text is meant to clarify that `read_only` being `false` doesn't mean *you* can actually write to the file, just that "in general" someone is able to.

Rollup of 9 pull requests Successful merges: - rust-lang#109263 (fix typo in documentation for std::fs::Permissions) - rust-lang#117156 (Convert `Unix{Datagram,Stream}::{set_}passcred()` to per-OS traits) - rust-lang#120684 (Update E0716.md for clarity) - rust-lang#121739 (Display short types for unimplemented trait) - rust-lang#121815 (Move `gather_comments`.) - rust-lang#121835 (Move `HandleStore` into `server.rs`.) - rust-lang#121847 (Remove hidden use of Global) - rust-lang#121861 (Use the guaranteed precision of a couple of float functions in docs) - rust-lang#121875 ( Account for unmet T: !Copy in E0277 message) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#109263 (fix typo in documentation for std::fs::Permissions) - rust-lang#120684 (Update E0716.md for clarity) - rust-lang#121715 (match lowering: pre-simplify or-patterns too) - rust-lang#121739 (Display short types for unimplemented trait) - rust-lang#121815 (Move `gather_comments`.) - rust-lang#121835 (Move `HandleStore` into `server.rs`.) - rust-lang#121847 (Remove hidden use of Global) - rust-lang#121861 (Use the guaranteed precision of a couple of float functions in docs) - rust-lang#121875 ( Account for unmet T: !Copy in E0277 message) r? `@ghost` `@rustbot` modify labels: rollup

fix typo in documentation for std::fs::Permissions Please check and re-check this PR carefully to see if I got this right. But by my logic, if the `read_only` function returns `true`, I would not expect be able to write to the file (it being read only); so this text is meant to clarify that `read_only` being `false` doesn't mean *you* can actually write to the file, just that "in general" someone is able to.

Rollup merge of rust-lang#109263 - squell:master, r=cuviper fix typo in documentation for std::fs::Permissions Please check and re-check this PR carefully to see if I got this right. But by my logic, if the `read_only` function returns `true`, I would not expect be able to write to the file (it being read only); so this text is meant to clarify that `read_only` being `false` doesn't mean *you* can actually write to the file, just that "in general" someone is able to.

fix typo in documentation for std::fs::Permissions

53469cf

rustbot assigned thomcc Mar 17, 2023

rustbot added 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 Mar 17, 2023

thomcc reviewed May 5, 2023

View reviewed changes

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels May 5, 2023

rewording after comments by @thomcc

0df002a

squell force-pushed the master branch from c7f0ad6 to 0df002a Compare August 29, 2023 09:24

Dylan-DPC added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 1, 2023

Noratrieb reviewed Jan 22, 2024

View reviewed changes

rustbot assigned cuviper and unassigned thomcc Feb 1, 2024

Amanieu 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 21, 2024

remove potentially misleading sentence about libc::access

b353765

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2024

matthiaskrgr mentioned this pull request Mar 2, 2024

Rollup of 10 pull requests #121886

Closed

Dylan-DPC mentioned this pull request Mar 2, 2024

Rollup of 9 pull requests #121887

Closed

matthiaskrgr mentioned this pull request Mar 2, 2024

Rollup of 9 pull requests #121890

Merged

matthiaskrgr mentioned this pull request Mar 2, 2024

Rollup of 10 pull requests #121896

Closed

bors merged commit d0e5431 into rust-lang:master Mar 2, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Mar 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix typo in documentation for std::fs::Permissions #109263

fix typo in documentation for std::fs::Permissions #109263

squell commented Mar 17, 2023

rustbot commented Mar 17, 2023

rustbot commented Mar 17, 2023

thomcc commented Mar 30, 2023

thomcc left a comment

thomcc May 5, 2023

squell May 7, 2023

thomcc commented May 5, 2023

Dylan-DPC commented Aug 7, 2023

squell commented Aug 29, 2023

Noratrieb Jan 22, 2024

squell Feb 8, 2024 •

edited

Loading

Noratrieb Feb 11, 2024

cuviper Feb 14, 2024

thomcc commented Feb 1, 2024

joshtriplett commented Feb 21, 2024

cuviper commented Mar 2, 2024

bors commented Mar 2, 2024

	/// is writable use the `access()` function from libc.
	/// is writable use the `access()` function from libc.

fix typo in documentation for std::fs::Permissions #109263

fix typo in documentation for std::fs::Permissions #109263

Conversation

squell commented Mar 17, 2023

rustbot commented Mar 17, 2023

rustbot commented Mar 17, 2023

thomcc commented Mar 30, 2023

thomcc left a comment

Choose a reason for hiding this comment

thomcc May 5, 2023

Choose a reason for hiding this comment

squell May 7, 2023

Choose a reason for hiding this comment

thomcc commented May 5, 2023

Dylan-DPC commented Aug 7, 2023

squell commented Aug 29, 2023

Noratrieb Jan 22, 2024

Choose a reason for hiding this comment

squell Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Noratrieb Feb 11, 2024

Choose a reason for hiding this comment

cuviper Feb 14, 2024

Choose a reason for hiding this comment

thomcc commented Feb 1, 2024

joshtriplett commented Feb 21, 2024

cuviper commented Mar 2, 2024

bors commented Mar 2, 2024

squell Feb 8, 2024 •

edited

Loading