-
Notifications
You must be signed in to change notification settings - Fork 38
tempfile: add new_with_modes and is_named methods #390
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.
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
base: main
Are you sure you want to change the base?
Conversation
f1e7b23
to
0c6abfb
Compare
Still pretty new to rust, not sure if this is ok to use |
cap-tempfile/src/tempfile.rs
Outdated
let mode = match mode { | ||
Some(mode) => Mode::from(mode), | ||
None => Mode::from(0o666), | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let mode = match mode { | |
Some(mode) => Mode::from(mode), | |
None => Mode::from(0o666), | |
}; | |
let mode = mode.unwrap_or(0o666).map(Mode::from); |
would be slightly shorter.
Actually for internal APIs I'd say we should actually use rustic::fs::Mode
as much as possible; in other words make this function take a Mode
instead. (We can't for public APIs yet as we aren't depending on rustix there yet AFAIK)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now using rustic::fs::Mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails for Darwin
error[E0308]: mismatched types
--> cap-tempfile/src/tempfile.rs:130:19
|
130 | opts.mode(mode.as_raw_mode());
| ---- ^^^^^^^^^^^^^^^^^^ expected `u32`, found `u16`
| |
| arguments to this method are incorrect
|
note: method defined here
--> /home/runner/work/cap-std/cap-std/cap-primitives/src/fs/open_options.rs:257:8
|
257 | fn mode(&mut self, mode: u32) -> &mut Self;
| ^^^^
help: you can convert a `u16` to a `u32`
|
130 | opts.mode(mode.as_raw_mode().into());
| +++++++
error[E0277]: the trait bound `Mode: From<u32>` is not satisfied
--> cap-tempfile/src/tempfile.rs:159:56
|
159 | let (fd, name) = new_tempfile(dir, false, Some(rustix::fs::Mode::from(mode)))?;
| ^^^^^^^^^^^^^^^^ the trait `From<u32>` is not implemented for `Mode`
|
= help: the trait `From<u32>` is not implemented for `Mode`
but trait `From<u16>` is implemented for it
= help: for that trait implementation, expected `u16`, found `u32`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to u32 for now
@@ -295,6 +325,29 @@ mod test { | |||
eprintln!("notice: Detected older Windows"); | |||
} | |||
|
|||
// Test that we can create with 0o000 mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit unusual, how about 0o200
instead so we can at least write to the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely unusual, but we can write to the fd as it was open with O_RDWR
242819e
to
62a9a6f
Compare
On filesystems that do not support `O_TMPFILE` (e.g. `vfat`), TempFile::new() automatically falls back to a potentially world readable named temp file. Add TempFile::new_with_modes() to create temp files with a chosen mode directly. We pass 2 modes, one for unnamed tempfile (happy path, created with `O_TMPFILE`), and one for named tempfile. The final mode is still restricted by the process umask.
62a9a6f
to
72ee5b6
Compare
new_with_mode
method
I switched to passing 2 modes (for unnamed and named case) + adding is_named() method. |
On filesystems that do not support
O_TMPFILE
(e.g.vfat
), TempFile::new()automatically falls back to a potentially world readable named temp file.
Add TempFile::new_with_modes() to create temp files with a chosen mode directly.
We pass 2 modes, one for unnamed tempfile (happy path, created with
O_TMPFILE
),and one for named tempfile.
The final mode is still restricted by the process umask.
@cgwalters this will help improve cap-std-ext atomic* permissions handling