From 42f4dd047af2de895df2754f7222b39f10cb6205 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Wed, 13 Jan 2016 18:08:08 +0100 Subject: [PATCH 1/5] Implement RFC 1252 expanding the OpenOptions structure Tracking issue: #30014 This implements the RFC and makes a few other changes. I have added a few extra tests, and made the Windows and Unix code as similar as possible. Part of the RFC mentions the unstable OpenOptionsExt trait on Windows (see #27720). I have added a few extra methods to future-proof it for CreateFile2. --- src/libstd/fs.rs | 226 ++++++++++++++++++++++++------- src/libstd/sys/unix/ext/fs.rs | 3 + src/libstd/sys/unix/fs.rs | 89 +++++++----- src/libstd/sys/windows/c.rs | 34 ++++- src/libstd/sys/windows/ext/fs.rs | 106 +++++++++++---- src/libstd/sys/windows/fs.rs | 156 ++++++++++++--------- 6 files changed, 437 insertions(+), 177 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index 635ed91f35da4..7b2555ff1f5ef 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -375,7 +375,7 @@ impl<'a> Seek for &'a File { } impl OpenOptions { - /// Creates a blank net set of options ready for configuration. + /// Creates a blank new set of options ready for configuration. /// /// All options are initially set to `false`. /// @@ -384,7 +384,8 @@ impl OpenOptions { /// ```no_run /// use std::fs::OpenOptions; /// - /// let file = OpenOptions::new().open("foo.txt"); + /// let mut options = OpenOptions::new(); + /// let file = options.read(true).open("foo.txt"); /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn new() -> OpenOptions { @@ -413,6 +414,9 @@ impl OpenOptions { /// This option, when true, will indicate that the file should be /// `write`-able if opened. /// + /// If a file already exist, the contents of that file get overwritten, but it is + /// not truncated. + /// /// # Examples /// /// ```no_run @@ -429,13 +433,29 @@ impl OpenOptions { /// /// This option, when true, means that writes will append to a file instead /// of overwriting previous contents. + /// Note that setting `.write(true).append(true)` has the same effect as + /// setting only `.append(true)`. + /// + /// For most filesystems the operating system guarantees all writes are atomic: + /// no writes get mangled because another process writes at the same time. + /// + /// One maybe obvious note when using append-mode: make sure that all data that + /// belongs together, is written the the file in one operation. This can be done + /// by concatenating strings before passing them to `write()`, or using a buffered + /// writer (with a more than adequately sized buffer) and calling `flush()` when the + /// message is complete. + /// + /// If a file is opened with both read and append access, beware that after opening + /// and after every write the position for reading may be set at the end of the file. + /// So before writing save the current position (using `seek(SeekFrom::Current(0))`, + /// and restore it before the next read. /// /// # Examples /// /// ```no_run /// use std::fs::OpenOptions; /// - /// let file = OpenOptions::new().write(true).append(true).open("foo.txt"); + /// let file = OpenOptions::new().append(true).open("foo.txt"); /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn append(&mut self, append: bool) -> &mut OpenOptions { @@ -447,6 +467,8 @@ impl OpenOptions { /// If a file is successfully opened with this option set it will truncate /// the file to 0 length if it already exists. /// + /// The file must be opened with write access for truncate to work. + /// /// # Examples /// /// ```no_run @@ -464,18 +486,79 @@ impl OpenOptions { /// This option indicates whether a new file will be created if the file /// does not yet already exist. /// + /// The file must be opened with write or append access in order to create + /// a new file. + /// /// # Examples /// /// ```no_run /// use std::fs::OpenOptions; /// - /// let file = OpenOptions::new().create(true).open("foo.txt"); + /// let file = OpenOptions::new().write(true).create(true).open("foo.txt"); /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn create(&mut self, create: bool) -> &mut OpenOptions { self.0.create(create); self } + /// Sets the option to always create a new file. + /// + /// This option indicates whether a new file will be created. + /// No file is allowed to exist at the target location, also no (dangling) + /// symlink. + /// + /// if `.create_new(true)` is set, `.create()` and `.truncate()` are ignored. + /// + /// The file must be opened with write or append access in order to create + /// a new file. + /// + /// # Examples + /// + /// ```no_run + /// use std::fs::OpenOptions; + /// + /// let file = OpenOptions::new().write(true).create_new(true).open("foo.txt"); + /// ``` + #[stable(feature = "expand_open_options", since = "1.7.0")] + pub fn create_new(&mut self, create_new: bool) -> &mut OpenOptions { + self.0.create_new(create_new); self + } + + /// Pass custom open flags to the operating system. + /// + /// Windows and the various flavours of Unix support flags that are not + /// cross-platform, but that can be useful in some circumstances. On Unix they will + /// be passed as the variable _flags_ to `open`, on Windows as the + /// _dwFlagsAndAttributes_ parameter. + /// + /// The cross-platform options of Rust can do magic: they can set any flag necessary + /// to ensure it works as expected. For example, `.append(true)` on Unix not only + /// sets the flag `O_APPEND`, but also automatically `O_WRONLY` or `O_RDWR`. This + /// special treatment is not available for the custom flags. + /// + /// Custom flags can only set flags, not remove flags set by Rusts options. + /// + /// For the custom flags on Unix, the bits that define the access mode are masked + /// out with `O_ACCMODE`, to ensure they do not interfere with the access mode set + /// by Rusts options. + /// + /// # Examples + /// + /// ```rust,ignore + /// extern crate libc; + /// extern crate winapi; + /// use std::fs::OpenOptions; + /// + /// let options = OpenOptions::new().write(true); + /// if cfg!(unix) { options.custom_flags(libc::O_NOFOLLOW); } + /// if cfg!(windows) { options.custom_flags(winapi::FILE_FLAG_BACKUP_SEMANTICS); } + /// let file = options.open("foo.txt"); + /// ``` + #[stable(feature = "expand_open_options", since = "1.7.0")] + pub fn custom_flags(&mut self, flags: u32) -> &mut OpenOptions { + self.0.custom_flags(flags); self + } + /// Opens a file at `path` with the options specified by `self`. /// /// # Errors @@ -483,10 +566,12 @@ impl OpenOptions { /// This function will return an error under a number of different /// circumstances, to include but not limited to: /// - /// * Opening a file that does not exist with read access. + /// * Opening a file that does not exist without setting `create` or `create_new`. /// * Attempting to open a file with access that the user lacks /// permissions for /// * Filesystem-level errors (full disk, etc) + /// * Invalid combinations of open options (truncate without write access, + /// no access mode set, etc) /// /// # Examples /// @@ -2098,61 +2183,108 @@ mod tests { let mut r = OO::new(); r.read(true); let mut w = OO::new(); w.write(true); - let mut rw = OO::new(); rw.write(true).read(true); - - match r.open(&tmpdir.join("a")) { - Ok(..) => panic!(), Err(..) => {} - } - - // Perform each one twice to make sure that it succeeds the second time - // (where the file exists) - check!(c(&w).create(true).open(&tmpdir.join("b"))); - assert!(tmpdir.join("b").exists()); - check!(c(&w).create(true).open(&tmpdir.join("b"))); - check!(w.open(&tmpdir.join("b"))); - - check!(c(&rw).create(true).open(&tmpdir.join("c"))); - assert!(tmpdir.join("c").exists()); + let mut rw = OO::new(); rw.read(true).write(true); + let mut a = OO::new(); a.append(true); + let mut ra = OO::new(); ra.read(true).append(true); + + let invalid_options = if cfg!(windows) { "The parameter is incorrect" } + else { "Invalid argument" }; + + // Test various combinations of creation modes and access modes. + // + // Allowed: + // creation mode | read | write | read-write | append | read-append | + // :-----------------------|:-----:|:-----:|:----------:|:------:|:-----------:| + // not set (open existing) | X | X | X | X | X | + // create | | X | X | X | X | + // truncate | | X | X | | | + // create and truncate | | X | X | | | + // create_new | | X | X | X | X | + // + // tested in reverse order, so 'create_new' creates the file, and 'open existing' opens it. + + // write-only + check!(c(&w).create_new(true).open(&tmpdir.join("a"))); + check!(c(&w).create(true).truncate(true).open(&tmpdir.join("a"))); + check!(c(&w).truncate(true).open(&tmpdir.join("a"))); + check!(c(&w).create(true).open(&tmpdir.join("a"))); + check!(c(&w).open(&tmpdir.join("a"))); + + // read-only + error!(c(&r).create_new(true).open(&tmpdir.join("b")), invalid_options); + error!(c(&r).create(true).truncate(true).open(&tmpdir.join("b")), invalid_options); + error!(c(&r).truncate(true).open(&tmpdir.join("b")), invalid_options); + error!(c(&r).create(true).open(&tmpdir.join("b")), invalid_options); + check!(c(&r).open(&tmpdir.join("a"))); // try opening the file created with write_only + + // read-write + check!(c(&rw).create_new(true).open(&tmpdir.join("c"))); + check!(c(&rw).create(true).truncate(true).open(&tmpdir.join("c"))); + check!(c(&rw).truncate(true).open(&tmpdir.join("c"))); check!(c(&rw).create(true).open(&tmpdir.join("c"))); - check!(rw.open(&tmpdir.join("c"))); - - check!(c(&w).append(true).create(true).open(&tmpdir.join("d"))); - assert!(tmpdir.join("d").exists()); - check!(c(&w).append(true).create(true).open(&tmpdir.join("d"))); - check!(c(&w).append(true).open(&tmpdir.join("d"))); - - check!(c(&rw).append(true).create(true).open(&tmpdir.join("e"))); - assert!(tmpdir.join("e").exists()); - check!(c(&rw).append(true).create(true).open(&tmpdir.join("e"))); - check!(c(&rw).append(true).open(&tmpdir.join("e"))); - - check!(c(&w).truncate(true).create(true).open(&tmpdir.join("f"))); - assert!(tmpdir.join("f").exists()); - check!(c(&w).truncate(true).create(true).open(&tmpdir.join("f"))); - check!(c(&w).truncate(true).open(&tmpdir.join("f"))); - - check!(c(&rw).truncate(true).create(true).open(&tmpdir.join("g"))); - assert!(tmpdir.join("g").exists()); - check!(c(&rw).truncate(true).create(true).open(&tmpdir.join("g"))); - check!(c(&rw).truncate(true).open(&tmpdir.join("g"))); - - check!(check!(File::create(&tmpdir.join("h"))).write("foo".as_bytes())); + check!(c(&rw).open(&tmpdir.join("c"))); + + // append + check!(c(&a).create_new(true).open(&tmpdir.join("d"))); + error!(c(&a).create(true).truncate(true).open(&tmpdir.join("d")), invalid_options); + error!(c(&a).truncate(true).open(&tmpdir.join("d")), invalid_options); + check!(c(&a).create(true).open(&tmpdir.join("d"))); + check!(c(&a).open(&tmpdir.join("d"))); + + // read-append + check!(c(&ra).create_new(true).open(&tmpdir.join("e"))); + error!(c(&ra).create(true).truncate(true).open(&tmpdir.join("e")), invalid_options); + error!(c(&ra).truncate(true).open(&tmpdir.join("e")), invalid_options); + check!(c(&ra).create(true).open(&tmpdir.join("e"))); + check!(c(&ra).open(&tmpdir.join("e"))); + + // Test opening a file without setting an access mode + let mut blank = OO::new(); + error!(blank.create(true).open(&tmpdir.join("f")), invalid_options); + + // Test write works + check!(check!(File::create(&tmpdir.join("h"))).write("foobar".as_bytes())); + + // Test write fails for read-only check!(r.open(&tmpdir.join("h"))); { let mut f = check!(r.open(&tmpdir.join("h"))); assert!(f.write("wut".as_bytes()).is_err()); } + + // Test write overwrites + { + let mut f = check!(c(&w).open(&tmpdir.join("h"))); + check!(f.write("baz".as_bytes())); + } + { + let mut f = check!(c(&r).open(&tmpdir.join("h"))); + let mut b = vec![0; 6]; + check!(f.read(&mut b)); + assert_eq!(b, "bazbar".as_bytes()); + } + + // Test truncate works + { + let mut f = check!(c(&w).truncate(true).open(&tmpdir.join("h"))); + check!(f.write("foo".as_bytes())); + } + assert_eq!(check!(fs::metadata(&tmpdir.join("h"))).len(), 3); + + // Test append works assert_eq!(check!(fs::metadata(&tmpdir.join("h"))).len(), 3); { - let mut f = check!(c(&w).append(true).open(&tmpdir.join("h"))); + let mut f = check!(c(&a).open(&tmpdir.join("h"))); check!(f.write("bar".as_bytes())); } assert_eq!(check!(fs::metadata(&tmpdir.join("h"))).len(), 6); + + // Test .append(true) equals .write(true).append(true) { - let mut f = check!(c(&w).truncate(true).open(&tmpdir.join("h"))); - check!(f.write("bar".as_bytes())); + let mut f = check!(c(&w).append(true).open(&tmpdir.join("h"))); + check!(f.write("baz".as_bytes())); } - assert_eq!(check!(fs::metadata(&tmpdir.join("h"))).len(), 3); + assert_eq!(check!(fs::metadata(&tmpdir.join("h"))).len(), 9); } #[test] diff --git a/src/libstd/sys/unix/ext/fs.rs b/src/libstd/sys/unix/ext/fs.rs index 16e1578296dfe..801b222b9a8c6 100644 --- a/src/libstd/sys/unix/ext/fs.rs +++ b/src/libstd/sys/unix/ext/fs.rs @@ -99,6 +99,9 @@ pub trait OpenOptionsExt { /// /// If a new file is created as part of a `File::open_opts` call then this /// specified `mode` will be used as the permission bits for the new file. + /// If no `mode` is set, the default of `0o666` will be used. + /// The operating system masks out bits with the systems `umask`, to produce + /// the final permissions. #[stable(feature = "fs_ext", since = "1.1.0")] fn mode(&mut self, mode: raw::mode_t) -> &mut Self; } diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index e8575a6c21cf1..c6a02f2b8b930 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -50,9 +50,15 @@ pub struct DirEntry { #[derive(Clone)] pub struct OpenOptions { - flags: c_int, + // generic read: bool, write: bool, + append: bool, + truncate: bool, + create: bool, + create_new: bool, + // system-specific + custom_flags: u32, mode: mode_t, } @@ -233,43 +239,58 @@ impl DirEntry { impl OpenOptions { pub fn new() -> OpenOptions { OpenOptions { - flags: libc::O_CLOEXEC, + // generic read: false, write: false, + append: false, + truncate: false, + create: false, + create_new: false, + // system-specific + custom_flags: 0, mode: 0o666, } } - pub fn read(&mut self, read: bool) { - self.read = read; - } - - pub fn write(&mut self, write: bool) { - self.write = write; - } - - pub fn append(&mut self, append: bool) { - self.flag(libc::O_APPEND, append); - } - - pub fn truncate(&mut self, truncate: bool) { - self.flag(libc::O_TRUNC, truncate); - } - - pub fn create(&mut self, create: bool) { - self.flag(libc::O_CREAT, create); - } - - pub fn mode(&mut self, mode: raw::mode_t) { - self.mode = mode as mode_t; + pub fn read(&mut self, read: bool) { self.read = read; } + pub fn write(&mut self, write: bool) { self.write = write; } + pub fn append(&mut self, append: bool) { self.append = append; } + pub fn truncate(&mut self, truncate: bool) { self.truncate = truncate; } + pub fn create(&mut self, create: bool) { self.create = create; } + pub fn create_new(&mut self, create_new: bool) { self.create_new = create_new; } + + pub fn custom_flags(&mut self, flags: u32) { self.custom_flags = flags; } + pub fn mode(&mut self, mode: raw::mode_t) { self.mode = mode as mode_t; } + + fn get_access_mode(&self) -> io::Result { + match (self.read, self.write, self.append) { + (true, false, false) => Ok(libc::O_RDONLY), + (false, true, false) => Ok(libc::O_WRONLY), + (true, true, false) => Ok(libc::O_RDWR), + (false, _, true) => Ok(libc::O_WRONLY | libc::O_APPEND), + (true, _, true) => Ok(libc::O_RDWR | libc::O_APPEND), + (false, false, false) => Err(Error::from_raw_os_error(libc::EINVAL)), + } } - fn flag(&mut self, bit: c_int, on: bool) { - if on { - self.flags |= bit; - } else { - self.flags &= !bit; + fn get_creation_mode(&self) -> io::Result { + match (self.write, self.append) { + (true, false) => {} + (false, false) => if self.truncate || self.create || self.create_new { + return Err(Error::from_raw_os_error(libc::EINVAL)); + }, + (_, true) => if self.truncate && !self.create_new { + return Err(Error::from_raw_os_error(libc::EINVAL)); + }, } + + Ok(match (self.create, self.truncate, self.create_new) { + (false, false, false) => 0, + (true, false, false) => libc::O_CREAT, + (false, true, false) => libc::O_TRUNC, + (true, true, false) => libc::O_CREAT | libc::O_TRUNC, + (_, _, true) => libc::O_CREAT | libc::O_EXCL, + }) } } @@ -280,12 +301,10 @@ impl File { } pub fn open_c(path: &CStr, opts: &OpenOptions) -> io::Result { - let flags = opts.flags | match (opts.read, opts.write) { - (true, true) => libc::O_RDWR, - (false, true) => libc::O_WRONLY, - (true, false) | - (false, false) => libc::O_RDONLY, - }; + let flags = libc::O_CLOEXEC | + try!(opts.get_access_mode()) | + try!(opts.get_creation_mode()) | + (opts.custom_flags as c_int & !libc::O_ACCMODE); let fd = try!(cvt_r(|| unsafe { libc::open(path.as_ptr(), flags, opts.mode as c_int) })); diff --git a/src/libstd/sys/windows/c.rs b/src/libstd/sys/windows/c.rs index 34e32d0d5b62a..6e8090a223516 100644 --- a/src/libstd/sys/windows/c.rs +++ b/src/libstd/sys/windows/c.rs @@ -93,16 +93,30 @@ pub const CREATE_NEW: DWORD = 1; pub const OPEN_ALWAYS: DWORD = 4; pub const OPEN_EXISTING: DWORD = 3; pub const TRUNCATE_EXISTING: DWORD = 5; -pub const FILE_APPEND_DATA: DWORD = 0x00000004; + pub const FILE_READ_DATA: DWORD = 0x00000001; pub const FILE_WRITE_DATA: DWORD = 0x00000002; -pub const STANDARD_RIGHTS_READ: DWORD = 0x20000; -pub const STANDARD_RIGHTS_WRITE: DWORD = 0x20000; -pub const FILE_WRITE_EA: DWORD = 0x00000010; +pub const FILE_APPEND_DATA: DWORD = 0x00000004; pub const FILE_READ_EA: DWORD = 0x00000008; -pub const SYNCHRONIZE: DWORD = 0x00100000; -pub const FILE_WRITE_ATTRIBUTES: DWORD = 0x00000100; +pub const FILE_WRITE_EA: DWORD = 0x00000010; +pub const FILE_EXECUTE: DWORD = 0x00000020; pub const FILE_READ_ATTRIBUTES: DWORD = 0x00000080; +pub const FILE_WRITE_ATTRIBUTES: DWORD = 0x00000100; + +pub const DELETE: DWORD = 0x00008000; +pub const READ_CONTROL: DWORD = 0x00020000; +pub const WRITE_DAC: DWORD = 0x00040000; +pub const WRITE_OWNER: DWORD = 0x00080000; +pub const SYNCHRONIZE: DWORD = 0x00100000; + +pub const GENERIC_READ: DWORD = 0x80000000; +pub const GENERIC_WRITE: DWORD = 0x40000000; +pub const GENERIC_EXECUTE: DWORD = 0x20000000; +pub const GENERIC_ALL: DWORD = 0x10000000; + +pub const STANDARD_RIGHTS_READ: DWORD = READ_CONTROL; +pub const STANDARD_RIGHTS_WRITE: DWORD = READ_CONTROL; +pub const STANDARD_RIGHTS_EXECUTE: DWORD = READ_CONTROL; pub const FILE_GENERIC_READ: DWORD = STANDARD_RIGHTS_READ | FILE_READ_DATA | FILE_READ_ATTRIBUTES | FILE_READ_EA | @@ -113,6 +127,14 @@ pub const FILE_GENERIC_WRITE: DWORD = STANDARD_RIGHTS_WRITE | FILE_WRITE_DATA | FILE_APPEND_DATA | SYNCHRONIZE; +pub const SECURITY_ANONYMOUS: DWORD = 0 << 16; +pub const SECURITY_IDENTIFICATION: DWORD = 1 << 16; +pub const SECURITY_IMPERSONATION: DWORD = 2 << 16; +pub const SECURITY_DELEGATION: DWORD = 3 << 16; +pub const SECURITY_CONTEXT_TRACKING: DWORD = 0x00040000; +pub const SECURITY_EFFECTIVE_ONLY: DWORD = 0x00080000; +pub const SECURITY_SQOS_PRESENT: DWORD = 0x00100000; + #[repr(C)] #[derive(Copy)] pub struct WIN32_FIND_DATAW { diff --git a/src/libstd/sys/windows/ext/fs.rs b/src/libstd/sys/windows/ext/fs.rs index 0d78d4de42b29..fa7e0e564ac25 100644 --- a/src/libstd/sys/windows/ext/fs.rs +++ b/src/libstd/sys/windows/ext/fs.rs @@ -25,45 +25,105 @@ use sys_common::{AsInnerMut, AsInner}; pub trait OpenOptionsExt { /// Overrides the `dwDesiredAccess` argument to the call to `CreateFile` /// with the specified value. - fn desired_access(&mut self, access: u32) -> &mut Self; - - /// Overrides the `dwCreationDisposition` argument to the call to - /// `CreateFile` with the specified value. /// - /// This will override any values of the standard `create` flags, for - /// example. - fn creation_disposition(&mut self, val: u32) -> &mut Self; - - /// Overrides the `dwFlagsAndAttributes` argument to the call to - /// `CreateFile` with the specified value. + /// This will override the `read`, `write`, and `append` flags on the + /// `OpenOptions` structure. This method provides fine-grained control + /// over the permissions to read, write and append data, attributes + /// (like hidden and system) and extended attributes. + /// + /// # Examples /// - /// This will override any values of the standard flags on the - /// `OpenOptions` structure. - fn flags_and_attributes(&mut self, val: u32) -> &mut Self; + /// ```no_run + /// #![feature(open_options_ext)] + /// use std::fs::OpenOptions; + /// use std::os::windows::fs::OpenOptionsExt; + /// + /// // Open without read and write permission, for example if you only need to call `stat()` + /// // on the file + /// let file = OpenOptions::new().access_mode(0).open("foo.txt"); + /// ``` + fn access_mode(&mut self, access: u32) -> &mut Self; /// Overrides the `dwShareMode` argument to the call to `CreateFile` with /// the specified value. /// - /// This will override any values of the standard flags on the - /// `OpenOptions` structure. + /// By default `share_mode` is set to `FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE`. + /// Specifying less permissions denies others to read from, write to and/or + /// delete the file while it is open. + /// + /// # Examples + /// + /// ```no_run + /// #![feature(open_options_ext)] + /// use std::fs::OpenOptions; + /// use std::os::windows::fs::OpenOptionsExt; + /// + /// let file = OpenOptions::new().write(true) + /// .share_mode(0) // Do not allow others to read or modify + /// .open("foo.txt"); + /// ``` fn share_mode(&mut self, val: u32) -> &mut Self; + + /// Sets the `dwFileAttributes` argument to the call to `CreateFile2` to + /// the specified value (or combines it with `custom_flags` and + /// `security_qos_flags` to set the `dwFlagsAndAttributes` for `CreateFile`). + /// + /// If a _new_ file is created because it does not yet exist and `.create(true)` or + /// `.create_new(true)` are specified, the new file is given the attributes declared + /// with `.attributes()`. + /// + /// If an _existing_ file is opened with `.create(true).truncate(true)`, its + /// existing attributes are preserved and combined with the ones declared with + /// `.attributes()`. + /// + /// In all other cases the attributes get ignored. + /// + /// # Examples + /// + /// ```rust,ignore + /// #![feature(open_options_ext)] + /// extern crate winapi; + /// use std::fs::OpenOptions; + /// use std::os::windows::fs::OpenOptionsExt; + /// + /// let file = OpenOptions::new().write(true).create(true) + /// .attributes(winapi::FILE_ATTRIBUTE_HIDDEN) + /// .open("foo.txt"); + /// ``` + fn attributes(&mut self, val: u32) -> &mut Self; + + /// Sets the `dwSecurityQosFlags` argument to the call to `CreateFile2` to + /// the specified value (or combines it with `custom_flags` and `attributes` + /// to set the `dwFlagsAndAttributes` for `CreateFile`). + fn security_qos_flags(&mut self, flags: u32) -> &mut OpenOptions; + + /// Sets the `lpSecurityAttributes` argument to the call to `CreateFile` to + /// the specified value. + fn security_attributes(&mut self, attrs: sys::c::LPSECURITY_ATTRIBUTES) -> &mut OpenOptions; } #[unstable(feature = "open_options_ext", reason = "may require more thought/methods", issue = "27720")] impl OpenOptionsExt for OpenOptions { - fn desired_access(&mut self, access: u32) -> &mut OpenOptions { - self.as_inner_mut().desired_access(access); self + fn access_mode(&mut self, access: u32) -> &mut OpenOptions { + self.as_inner_mut().access_mode(access); self } - fn creation_disposition(&mut self, access: u32) -> &mut OpenOptions { - self.as_inner_mut().creation_disposition(access); self + + fn share_mode(&mut self, share: u32) -> &mut OpenOptions { + self.as_inner_mut().share_mode(share); self } - fn flags_and_attributes(&mut self, access: u32) -> &mut OpenOptions { - self.as_inner_mut().flags_and_attributes(access); self + + fn attributes(&mut self, attributes: u32) -> &mut OpenOptions { + self.as_inner_mut().attributes(attributes); self } - fn share_mode(&mut self, access: u32) -> &mut OpenOptions { - self.as_inner_mut().share_mode(access); self + + fn security_qos_flags(&mut self, flags: u32) -> &mut OpenOptions { + self.as_inner_mut().security_qos_flags(flags); self + } + + fn security_attributes(&mut self, attrs: sys::c::LPSECURITY_ATTRIBUTES) -> &mut OpenOptions { + self.as_inner_mut().security_attributes(attrs); self } } diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index 9db7ab534590a..b3b80be208b66 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -54,18 +54,22 @@ pub struct DirEntry { data: c::WIN32_FIND_DATAW, } -#[derive(Clone, Default)] +#[derive(Clone)] pub struct OpenOptions { - create: bool, - append: bool, + // generic read: bool, write: bool, + append: bool, truncate: bool, - desired_access: Option, - share_mode: Option, - creation_disposition: Option, - flags_and_attributes: Option, - security_attributes: usize, // *mut T doesn't have a Default impl + create: bool, + create_new: bool, + // system-specific + custom_flags: u32, + access_mode: Option, + attributes: c::DWORD, + share_mode: c::DWORD, + security_qos_flags: c::DWORD, + security_attributes: c::LPSECURITY_ATTRIBUTES, } #[derive(Clone, PartialEq, Eq, Debug)] @@ -151,68 +155,84 @@ impl DirEntry { } impl OpenOptions { - pub fn new() -> OpenOptions { Default::default() } + pub fn new() -> OpenOptions { + OpenOptions { + // generic + read: false, + write: false, + append: false, + truncate: false, + create: false, + create_new: false, + // system-specific + custom_flags: 0, + access_mode: None, + share_mode: c::FILE_SHARE_READ | c::FILE_SHARE_WRITE | c::FILE_SHARE_DELETE, + attributes: 0, + security_qos_flags: 0, + security_attributes: ptr::null_mut(), + } + } + pub fn read(&mut self, read: bool) { self.read = read; } pub fn write(&mut self, write: bool) { self.write = write; } pub fn append(&mut self, append: bool) { self.append = append; } - pub fn create(&mut self, create: bool) { self.create = create; } pub fn truncate(&mut self, truncate: bool) { self.truncate = truncate; } - pub fn creation_disposition(&mut self, val: u32) { - self.creation_disposition = Some(val); - } - pub fn flags_and_attributes(&mut self, val: u32) { - self.flags_and_attributes = Some(val); - } - pub fn desired_access(&mut self, val: u32) { - self.desired_access = Some(val); - } - pub fn share_mode(&mut self, val: u32) { - self.share_mode = Some(val); - } + pub fn create(&mut self, create: bool) { self.create = create; } + pub fn create_new(&mut self, create_new: bool) { self.create_new = create_new; } + + pub fn custom_flags(&mut self, flags: u32) { self.custom_flags = flags; } + pub fn access_mode(&mut self, access_mode: u32) { self.access_mode = Some(access_mode); } + pub fn share_mode(&mut self, share_mode: u32) { self.share_mode = share_mode; } + pub fn attributes(&mut self, attrs: u32) { self.attributes = attrs; } + pub fn security_qos_flags(&mut self, flags: u32) { self.security_qos_flags = flags; } pub fn security_attributes(&mut self, attrs: c::LPSECURITY_ATTRIBUTES) { - self.security_attributes = attrs as usize; + self.security_attributes = attrs; } - fn get_desired_access(&self) -> c::DWORD { - self.desired_access.unwrap_or({ - let mut base = if self.read {c::FILE_GENERIC_READ} else {0} | - if self.write {c::FILE_GENERIC_WRITE} else {0}; - if self.append { - base &= !c::FILE_WRITE_DATA; - base |= c::FILE_APPEND_DATA; - } - base - }) + fn get_access_mode(&self) -> io::Result { + const ERROR_INVALID_PARAMETER: i32 = 87; + + match (self.read, self.write, self.append, self.access_mode) { + (_, _, _, Some(mode)) => Ok(mode), + (true, false, false, None) => Ok(c::GENERIC_READ), + (false, true, false, None) => Ok(c::GENERIC_WRITE), + (true, true, false, None) => Ok(c::GENERIC_READ | c::GENERIC_WRITE), + (false, _, true, None) => Ok(c::FILE_GENERIC_WRITE & !c::FILE_WRITE_DATA), + (true, _, true, None) => Ok(c::GENERIC_READ | + (c::FILE_GENERIC_WRITE & !c::FILE_WRITE_DATA)), + (false, false, false, None) => Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)), + } } - fn get_share_mode(&self) -> c::DWORD { - // libuv has a good comment about this, but the basic idea is that - // we try to emulate unix semantics by enabling all sharing by - // allowing things such as deleting a file while it's still open. - self.share_mode.unwrap_or(c::FILE_SHARE_READ | - c::FILE_SHARE_WRITE | - c::FILE_SHARE_DELETE) - } - - fn get_creation_disposition(&self) -> c::DWORD { - self.creation_disposition.unwrap_or({ - match (self.create, self.truncate) { - (true, true) => c::CREATE_ALWAYS, - (true, false) => c::OPEN_ALWAYS, - (false, false) => c::OPEN_EXISTING, - (false, true) => { - if self.write && !self.append { - c::CREATE_ALWAYS - } else { - c::TRUNCATE_EXISTING - } - } - } - }) + fn get_creation_mode(&self) -> io::Result { + const ERROR_INVALID_PARAMETER: i32 = 87; + + match (self.write, self.append) { + (true, false) => {} + (false, false) => if self.truncate || self.create || self.create_new { + return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); + }, + (_, true) => if self.truncate && !self.create_new { + return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); + }, + } + + Ok(match (self.create, self.truncate, self.create_new) { + (false, false, false) => c::OPEN_EXISTING, + (true, false, false) => c::OPEN_ALWAYS, + (false, true, false) => c::TRUNCATE_EXISTING, + (true, true, false) => c::CREATE_ALWAYS, + (_, _, true) => c::CREATE_NEW, + }) } fn get_flags_and_attributes(&self) -> c::DWORD { - self.flags_and_attributes.unwrap_or(c::FILE_ATTRIBUTE_NORMAL) + self.custom_flags | + self.attributes | + self.security_qos_flags | + if self.security_qos_flags != 0 { c::SECURITY_SQOS_PRESENT } else { 0 } | + if self.create_new { c::FILE_FLAG_OPEN_REPARSE_POINT } else { 0 } } } @@ -221,8 +241,8 @@ impl File { let mut opts = OpenOptions::new(); opts.read(!write); opts.write(write); - opts.flags_and_attributes(c::FILE_FLAG_OPEN_REPARSE_POINT | - c::FILE_FLAG_BACKUP_SEMANTICS); + opts.custom_flags(c::FILE_FLAG_OPEN_REPARSE_POINT | + c::FILE_FLAG_BACKUP_SEMANTICS); File::open(path, &opts) } @@ -230,10 +250,10 @@ impl File { let path = try!(to_u16s(path)); let handle = unsafe { c::CreateFileW(path.as_ptr(), - opts.get_desired_access(), - opts.get_share_mode(), + try!(opts.get_access_mode()), + opts.share_mode, opts.security_attributes as *mut _, - opts.get_creation_disposition(), + try!(opts.get_creation_mode()), opts.get_flags_and_attributes(), ptr::null_mut()) }; @@ -533,7 +553,10 @@ pub fn stat(p: &Path) -> io::Result { // metadata information is. if attr.is_reparse_point() { let mut opts = OpenOptions::new(); - opts.flags_and_attributes(c::FILE_FLAG_BACKUP_SEMANTICS); + // No read or write permissions are necessary + opts.access_mode(0); + // This flag is so we can open directories too + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS); let file = try!(File::open(p, &opts)); file.file_attr() } else { @@ -577,9 +600,10 @@ fn get_path(f: &File) -> io::Result { pub fn canonicalize(p: &Path) -> io::Result { let mut opts = OpenOptions::new(); - opts.read(true); + // No read or write permissions are necessary + opts.access_mode(0); // This flag is so we can open directories too - opts.flags_and_attributes(c::FILE_FLAG_BACKUP_SEMANTICS); + opts.custom_flags(c::FILE_FLAG_BACKUP_SEMANTICS); let f = try!(File::open(p, &opts)); get_path(&f) } From 7a1817c9d4a0649a7ea1c948bd731a5b63aa3d06 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Wed, 13 Jan 2016 21:47:46 +0100 Subject: [PATCH 2/5] Move `custom_flags` to `OpenOptionsExt` And mark the new methods as unstable. --- src/libstd/fs.rs | 39 +++----------------------------- src/libstd/sys/unix/ext/fs.rs | 28 ++++++++++++++++++++++- src/libstd/sys/unix/fs.rs | 4 ++-- src/libstd/sys/windows/ext/fs.rs | 26 +++++++++++++++++++++ 4 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index 7b2555ff1f5ef..6a96ab56fc341 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -519,46 +519,13 @@ impl OpenOptions { /// /// let file = OpenOptions::new().write(true).create_new(true).open("foo.txt"); /// ``` - #[stable(feature = "expand_open_options", since = "1.7.0")] + #[unstable(feature = "expand_open_options", + reason = "recently added", + issue = "30014")] pub fn create_new(&mut self, create_new: bool) -> &mut OpenOptions { self.0.create_new(create_new); self } - /// Pass custom open flags to the operating system. - /// - /// Windows and the various flavours of Unix support flags that are not - /// cross-platform, but that can be useful in some circumstances. On Unix they will - /// be passed as the variable _flags_ to `open`, on Windows as the - /// _dwFlagsAndAttributes_ parameter. - /// - /// The cross-platform options of Rust can do magic: they can set any flag necessary - /// to ensure it works as expected. For example, `.append(true)` on Unix not only - /// sets the flag `O_APPEND`, but also automatically `O_WRONLY` or `O_RDWR`. This - /// special treatment is not available for the custom flags. - /// - /// Custom flags can only set flags, not remove flags set by Rusts options. - /// - /// For the custom flags on Unix, the bits that define the access mode are masked - /// out with `O_ACCMODE`, to ensure they do not interfere with the access mode set - /// by Rusts options. - /// - /// # Examples - /// - /// ```rust,ignore - /// extern crate libc; - /// extern crate winapi; - /// use std::fs::OpenOptions; - /// - /// let options = OpenOptions::new().write(true); - /// if cfg!(unix) { options.custom_flags(libc::O_NOFOLLOW); } - /// if cfg!(windows) { options.custom_flags(winapi::FILE_FLAG_BACKUP_SEMANTICS); } - /// let file = options.open("foo.txt"); - /// ``` - #[stable(feature = "expand_open_options", since = "1.7.0")] - pub fn custom_flags(&mut self, flags: u32) -> &mut OpenOptions { - self.0.custom_flags(flags); self - } - /// Opens a file at `path` with the options specified by `self`. /// /// # Errors diff --git a/src/libstd/sys/unix/ext/fs.rs b/src/libstd/sys/unix/ext/fs.rs index 801b222b9a8c6..75ea5846fb3e7 100644 --- a/src/libstd/sys/unix/ext/fs.rs +++ b/src/libstd/sys/unix/ext/fs.rs @@ -104,6 +104,29 @@ pub trait OpenOptionsExt { /// the final permissions. #[stable(feature = "fs_ext", since = "1.1.0")] fn mode(&mut self, mode: raw::mode_t) -> &mut Self; + + /// Pass custom flags to the `flags` agument of `open`. + /// + /// The bits that define the access mode are masked out with `O_ACCMODE`, to ensure + /// they do not interfere with the access mode set by Rusts options. + /// + /// Custom flags can only set flags, not remove flags set by Rusts options. + /// + /// # Examples + /// + /// ```no_run + /// extern crate libc; + /// use std::fs::OpenOptions; + /// use std::os::unix::fs::OpenOptionsExt; + /// + /// let options = OpenOptions::new().write(true); + /// if cfg!(unix) { options.custom_flags(libc::O_NOFOLLOW); } + /// let file = options.open("foo.txt"); + /// ``` + #[unstable(feature = "expand_open_options", + reason = "recently added", + issue = "30014")] + fn custom_flags(&mut self, flags: i32) -> &mut Self; } #[stable(feature = "fs_ext", since = "1.1.0")] @@ -111,6 +134,10 @@ impl OpenOptionsExt for OpenOptions { fn mode(&mut self, mode: raw::mode_t) -> &mut OpenOptions { self.as_inner_mut().mode(mode); self } + + fn custom_flags(&mut self, flags: i32) -> &mut OpenOptions { + self.as_inner_mut().custom_flags(flags); self + } } // Hm, why are there casts here to the returned type, shouldn't the types always @@ -265,4 +292,3 @@ impl DirBuilderExt for fs::DirBuilder { self } } - diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index c6a02f2b8b930..31ea6b7dd2e49 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -58,7 +58,7 @@ pub struct OpenOptions { create: bool, create_new: bool, // system-specific - custom_flags: u32, + custom_flags: i32, mode: mode_t, } @@ -259,7 +259,7 @@ impl OpenOptions { pub fn create(&mut self, create: bool) { self.create = create; } pub fn create_new(&mut self, create_new: bool) { self.create_new = create_new; } - pub fn custom_flags(&mut self, flags: u32) { self.custom_flags = flags; } + pub fn custom_flags(&mut self, flags: i32) { self.custom_flags = flags; } pub fn mode(&mut self, mode: raw::mode_t) { self.mode = mode as mode_t; } fn get_access_mode(&self) -> io::Result { diff --git a/src/libstd/sys/windows/ext/fs.rs b/src/libstd/sys/windows/ext/fs.rs index fa7e0e564ac25..04053b6cb6a62 100644 --- a/src/libstd/sys/windows/ext/fs.rs +++ b/src/libstd/sys/windows/ext/fs.rs @@ -64,6 +64,28 @@ pub trait OpenOptionsExt { /// ``` fn share_mode(&mut self, val: u32) -> &mut Self; + /// Sets extra flags for the `dwFileFlags` argument to the call to `CreateFile2` + /// (or combines it with `attributes` and `security_qos_flags` to set the + /// `dwFlagsAndAttributes` for `CreateFile`). + /// + /// Custom flags can only set flags, not remove flags set by Rusts options. + /// + /// # Examples + /// + /// ```rust,ignore + /// extern crate winapi; + /// use std::fs::OpenOptions; + /// use std::os::windows::fs::OpenOptionsExt; + /// + /// let options = OpenOptions::new().create(true).write(true); + /// if cfg!(windows) { options.custom_flags(winapi::FILE_FLAG_DELETE_ON_CLOSE); } + /// let file = options.open("foo.txt"); + /// ``` + #[unstable(feature = "expand_open_options", + reason = "recently added", + issue = "30014")] + fn custom_flags(&mut self, flags: u32) -> &mut Self; + /// Sets the `dwFileAttributes` argument to the call to `CreateFile2` to /// the specified value (or combines it with `custom_flags` and /// `security_qos_flags` to set the `dwFlagsAndAttributes` for `CreateFile`). @@ -114,6 +136,10 @@ impl OpenOptionsExt for OpenOptions { self.as_inner_mut().share_mode(share); self } + fn custom_flags(&mut self, flags: u32) -> &mut OpenOptions { + self.as_inner_mut().custom_flags(flags); self + } + fn attributes(&mut self, attributes: u32) -> &mut OpenOptions { self.as_inner_mut().attributes(attributes); self } From 1230a08679e70b8d9a9ce653f8663ff27832db54 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Thu, 14 Jan 2016 16:59:28 +0100 Subject: [PATCH 3/5] Fix doctests --- src/libstd/fs.rs | 1 + src/libstd/sys/unix/ext/fs.rs | 5 +++-- src/libstd/sys/windows/ext/fs.rs | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index 6a96ab56fc341..fcaa2fede2e87 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -515,6 +515,7 @@ impl OpenOptions { /// # Examples /// /// ```no_run + /// #![feature(expand_open_options)] /// use std::fs::OpenOptions; /// /// let file = OpenOptions::new().write(true).create_new(true).open("foo.txt"); diff --git a/src/libstd/sys/unix/ext/fs.rs b/src/libstd/sys/unix/ext/fs.rs index 75ea5846fb3e7..33f29d44f87b6 100644 --- a/src/libstd/sys/unix/ext/fs.rs +++ b/src/libstd/sys/unix/ext/fs.rs @@ -114,12 +114,13 @@ pub trait OpenOptionsExt { /// /// # Examples /// - /// ```no_run + /// ```rust,ignore /// extern crate libc; /// use std::fs::OpenOptions; /// use std::os::unix::fs::OpenOptionsExt; /// - /// let options = OpenOptions::new().write(true); + /// let mut options = OpenOptions::new(); + /// options.write(true); /// if cfg!(unix) { options.custom_flags(libc::O_NOFOLLOW); } /// let file = options.open("foo.txt"); /// ``` diff --git a/src/libstd/sys/windows/ext/fs.rs b/src/libstd/sys/windows/ext/fs.rs index 04053b6cb6a62..4bb67b0fad953 100644 --- a/src/libstd/sys/windows/ext/fs.rs +++ b/src/libstd/sys/windows/ext/fs.rs @@ -77,7 +77,8 @@ pub trait OpenOptionsExt { /// use std::fs::OpenOptions; /// use std::os::windows::fs::OpenOptionsExt; /// - /// let options = OpenOptions::new().create(true).write(true); + /// let mut options = OpenOptions::new(); + /// options.create(true).write(true); /// if cfg!(windows) { options.custom_flags(winapi::FILE_FLAG_DELETE_ON_CLOSE); } /// let file = options.open("foo.txt"); /// ``` From 9c569189c8b8b5d635b6704c489a8410c81570a0 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Fri, 15 Jan 2016 19:04:53 +0100 Subject: [PATCH 4/5] Addressed comments --- src/libstd/fs.rs | 41 ++++++++++++++---------- src/libstd/sys/unix/ext/fs.rs | 9 ++++-- src/libstd/sys/unix/fs.rs | 16 +++++----- src/libstd/sys/windows/ext/fs.rs | 53 ++++++++++++++++---------------- src/libstd/sys/windows/fs.rs | 16 +++++----- 5 files changed, 75 insertions(+), 60 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index fcaa2fede2e87..40c08b1768004 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -414,8 +414,8 @@ impl OpenOptions { /// This option, when true, will indicate that the file should be /// `write`-able if opened. /// - /// If a file already exist, the contents of that file get overwritten, but it is - /// not truncated. + /// If a file already exist, any write calls on the file will overwrite its + /// contents, without truncating it. /// /// # Examples /// @@ -436,19 +436,20 @@ impl OpenOptions { /// Note that setting `.write(true).append(true)` has the same effect as /// setting only `.append(true)`. /// - /// For most filesystems the operating system guarantees all writes are atomic: - /// no writes get mangled because another process writes at the same time. + /// For most filesystems the operating system guarantees all writes are + /// atomic: no writes get mangled because another process writes at the same + /// time. /// - /// One maybe obvious note when using append-mode: make sure that all data that - /// belongs together, is written the the file in one operation. This can be done - /// by concatenating strings before passing them to `write()`, or using a buffered - /// writer (with a more than adequately sized buffer) and calling `flush()` when the - /// message is complete. + /// One maybe obvious note when using append-mode: make sure that all data + /// that belongs together, is written the the file in one operation. This + /// can be done by concatenating strings before passing them to `write()`, + /// or using a buffered writer (with a more than adequately sized buffer) + /// and calling `flush()` when the message is complete. /// - /// If a file is opened with both read and append access, beware that after opening - /// and after every write the position for reading may be set at the end of the file. - /// So before writing save the current position (using `seek(SeekFrom::Current(0))`, - /// and restore it before the next read. + /// If a file is opened with both read and append access, beware that after + /// opening and after every write the position for reading may be set at the + /// end of the file. So before writing save the current position (using + /// `seek(SeekFrom::Current(0))`, and restore it before the next read. /// /// # Examples /// @@ -507,7 +508,12 @@ impl OpenOptions { /// No file is allowed to exist at the target location, also no (dangling) /// symlink. /// - /// if `.create_new(true)` is set, `.create()` and `.truncate()` are ignored. + /// This option is usefull because it as atomic. Otherwise between checking + /// whether a file exists and creating a new one, the file may have been + /// created by another process (a TOCTOU race condition / attack). + /// + /// If `.create_new(true)` is set, `.create()` and `.truncate()` are + /// ignored. /// /// The file must be opened with write or append access in order to create /// a new file. @@ -518,7 +524,9 @@ impl OpenOptions { /// #![feature(expand_open_options)] /// use std::fs::OpenOptions; /// - /// let file = OpenOptions::new().write(true).create_new(true).open("foo.txt"); + /// let file = OpenOptions::new().write(true) + /// .create_new(true) + /// .open("foo.txt"); /// ``` #[unstable(feature = "expand_open_options", reason = "recently added", @@ -534,7 +542,8 @@ impl OpenOptions { /// This function will return an error under a number of different /// circumstances, to include but not limited to: /// - /// * Opening a file that does not exist without setting `create` or `create_new`. + /// * Opening a file that does not exist without setting `create` or + /// `create_new`. /// * Attempting to open a file with access that the user lacks /// permissions for /// * Filesystem-level errors (full disk, etc) diff --git a/src/libstd/sys/unix/ext/fs.rs b/src/libstd/sys/unix/ext/fs.rs index 33f29d44f87b6..46416753c02a7 100644 --- a/src/libstd/sys/unix/ext/fs.rs +++ b/src/libstd/sys/unix/ext/fs.rs @@ -107,10 +107,11 @@ pub trait OpenOptionsExt { /// Pass custom flags to the `flags` agument of `open`. /// - /// The bits that define the access mode are masked out with `O_ACCMODE`, to ensure - /// they do not interfere with the access mode set by Rusts options. + /// The bits that define the access mode are masked out with `O_ACCMODE`, to + /// ensure they do not interfere with the access mode set by Rusts options. /// /// Custom flags can only set flags, not remove flags set by Rusts options. + /// This options overwrites any previously set custom flags. /// /// # Examples /// @@ -121,7 +122,9 @@ pub trait OpenOptionsExt { /// /// let mut options = OpenOptions::new(); /// options.write(true); - /// if cfg!(unix) { options.custom_flags(libc::O_NOFOLLOW); } + /// if cfg!(unix) { + /// options.custom_flags(libc::O_NOFOLLOW); + /// } /// let file = options.open("foo.txt"); /// ``` #[unstable(feature = "expand_open_options", diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 31ea6b7dd2e49..1e5dc972bc533 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -275,13 +275,15 @@ impl OpenOptions { fn get_creation_mode(&self) -> io::Result { match (self.write, self.append) { - (true, false) => {} - (false, false) => if self.truncate || self.create || self.create_new { - return Err(Error::from_raw_os_error(libc::EINVAL)); - }, - (_, true) => if self.truncate && !self.create_new { - return Err(Error::from_raw_os_error(libc::EINVAL)); - }, + (true, false) => {} + (false, false) => + if self.truncate || self.create || self.create_new { + return Err(Error::from_raw_os_error(libc::EINVAL)); + }, + (_, true) => + if self.truncate && !self.create_new { + return Err(Error::from_raw_os_error(libc::EINVAL)); + }, } Ok(match (self.create, self.truncate, self.create_new) { diff --git a/src/libstd/sys/windows/ext/fs.rs b/src/libstd/sys/windows/ext/fs.rs index 4bb67b0fad953..d060c902fba2f 100644 --- a/src/libstd/sys/windows/ext/fs.rs +++ b/src/libstd/sys/windows/ext/fs.rs @@ -27,9 +27,9 @@ pub trait OpenOptionsExt { /// with the specified value. /// /// This will override the `read`, `write`, and `append` flags on the - /// `OpenOptions` structure. This method provides fine-grained control - /// over the permissions to read, write and append data, attributes - /// (like hidden and system) and extended attributes. + /// `OpenOptions` structure. This method provides fine-grained control over + /// the permissions to read, write and append data, attributes (like hidden + /// and system) and extended attributes. /// /// # Examples /// @@ -38,8 +38,8 @@ pub trait OpenOptionsExt { /// use std::fs::OpenOptions; /// use std::os::windows::fs::OpenOptionsExt; /// - /// // Open without read and write permission, for example if you only need to call `stat()` - /// // on the file + /// // Open without read and write permission, for example if you only need + /// // to call `stat()` on the file /// let file = OpenOptions::new().access_mode(0).open("foo.txt"); /// ``` fn access_mode(&mut self, access: u32) -> &mut Self; @@ -47,9 +47,10 @@ pub trait OpenOptionsExt { /// Overrides the `dwShareMode` argument to the call to `CreateFile` with /// the specified value. /// - /// By default `share_mode` is set to `FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE`. - /// Specifying less permissions denies others to read from, write to and/or - /// delete the file while it is open. + /// By default `share_mode` is set to + /// `FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE`. Specifying + /// less permissions denies others to read from, write to and/or delete the + /// file while it is open. /// /// # Examples /// @@ -58,17 +59,20 @@ pub trait OpenOptionsExt { /// use std::fs::OpenOptions; /// use std::os::windows::fs::OpenOptionsExt; /// + /// // Do not allow others to read or modify this file while we have it open + /// // for writing /// let file = OpenOptions::new().write(true) - /// .share_mode(0) // Do not allow others to read or modify + /// .share_mode(0) /// .open("foo.txt"); /// ``` fn share_mode(&mut self, val: u32) -> &mut Self; - /// Sets extra flags for the `dwFileFlags` argument to the call to `CreateFile2` - /// (or combines it with `attributes` and `security_qos_flags` to set the - /// `dwFlagsAndAttributes` for `CreateFile`). + /// Sets extra flags for the `dwFileFlags` argument to the call to + /// `CreateFile2` (or combines it with `attributes` and `security_qos_flags` + /// to set the `dwFlagsAndAttributes` for `CreateFile`). /// /// Custom flags can only set flags, not remove flags set by Rusts options. + /// This options overwrites any previously set custom flags. /// /// # Examples /// @@ -79,7 +83,9 @@ pub trait OpenOptionsExt { /// /// let mut options = OpenOptions::new(); /// options.create(true).write(true); - /// if cfg!(windows) { options.custom_flags(winapi::FILE_FLAG_DELETE_ON_CLOSE); } + /// if cfg!(windows) { + /// options.custom_flags(winapi::FILE_FLAG_DELETE_ON_CLOSE); + /// } /// let file = options.open("foo.txt"); /// ``` #[unstable(feature = "expand_open_options", @@ -89,15 +95,16 @@ pub trait OpenOptionsExt { /// Sets the `dwFileAttributes` argument to the call to `CreateFile2` to /// the specified value (or combines it with `custom_flags` and - /// `security_qos_flags` to set the `dwFlagsAndAttributes` for `CreateFile`). + /// `security_qos_flags` to set the `dwFlagsAndAttributes` for + /// `CreateFile`). /// - /// If a _new_ file is created because it does not yet exist and `.create(true)` or - /// `.create_new(true)` are specified, the new file is given the attributes declared - /// with `.attributes()`. + /// If a _new_ file is created because it does not yet exist and + ///`.create(true)` or `.create_new(true)` are specified, the new file is + /// given the attributes declared with `.attributes()`. /// /// If an _existing_ file is opened with `.create(true).truncate(true)`, its - /// existing attributes are preserved and combined with the ones declared with - /// `.attributes()`. + /// existing attributes are preserved and combined with the ones declared + /// with `.attributes()`. /// /// In all other cases the attributes get ignored. /// @@ -119,10 +126,6 @@ pub trait OpenOptionsExt { /// the specified value (or combines it with `custom_flags` and `attributes` /// to set the `dwFlagsAndAttributes` for `CreateFile`). fn security_qos_flags(&mut self, flags: u32) -> &mut OpenOptions; - - /// Sets the `lpSecurityAttributes` argument to the call to `CreateFile` to - /// the specified value. - fn security_attributes(&mut self, attrs: sys::c::LPSECURITY_ATTRIBUTES) -> &mut OpenOptions; } #[unstable(feature = "open_options_ext", @@ -148,10 +151,6 @@ impl OpenOptionsExt for OpenOptions { fn security_qos_flags(&mut self, flags: u32) -> &mut OpenOptions { self.as_inner_mut().security_qos_flags(flags); self } - - fn security_attributes(&mut self, attrs: sys::c::LPSECURITY_ATTRIBUTES) -> &mut OpenOptions { - self.as_inner_mut().security_attributes(attrs); self - } } /// Extension methods for `fs::Metadata` to access the raw fields contained diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index b3b80be208b66..df0a7bda18d73 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -209,13 +209,15 @@ impl OpenOptions { const ERROR_INVALID_PARAMETER: i32 = 87; match (self.write, self.append) { - (true, false) => {} - (false, false) => if self.truncate || self.create || self.create_new { - return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); - }, - (_, true) => if self.truncate && !self.create_new { - return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); - }, + (true, false) => {} + (false, false) => + if self.truncate || self.create || self.create_new { + return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); + }, + (_, true) => + if self.truncate && !self.create_new { + return Err(Error::from_raw_os_error(ERROR_INVALID_PARAMETER)); + }, } Ok(match (self.create, self.truncate, self.create_new) { From ae30294771e3c5c65a2d70be0e09b5bec2490c66 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Wed, 20 Jan 2016 08:41:20 +0100 Subject: [PATCH 5/5] Remove raw pointer from OpenOptions struct Otherwise it is not Send and Sync anymore --- src/libstd/fs.rs | 6 ++++++ src/libstd/sys/windows/fs.rs | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index 40c08b1768004..414a0ebd11fa2 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -2264,6 +2264,12 @@ mod tests { assert_eq!(check!(fs::metadata(&tmpdir.join("h"))).len(), 9); } + #[test] + fn _assert_send_sync() { + fn _assert_send_sync() {} + _assert_send_sync::(); + } + #[test] fn binary_file() { let mut bytes = [0; 1024]; diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index df0a7bda18d73..60e3f7c22bd58 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -69,7 +69,7 @@ pub struct OpenOptions { attributes: c::DWORD, share_mode: c::DWORD, security_qos_flags: c::DWORD, - security_attributes: c::LPSECURITY_ATTRIBUTES, + security_attributes: usize, // FIXME: should be a reference } #[derive(Clone, PartialEq, Eq, Debug)] @@ -170,7 +170,7 @@ impl OpenOptions { share_mode: c::FILE_SHARE_READ | c::FILE_SHARE_WRITE | c::FILE_SHARE_DELETE, attributes: 0, security_qos_flags: 0, - security_attributes: ptr::null_mut(), + security_attributes: 0, } } @@ -187,7 +187,7 @@ impl OpenOptions { pub fn attributes(&mut self, attrs: u32) { self.attributes = attrs; } pub fn security_qos_flags(&mut self, flags: u32) { self.security_qos_flags = flags; } pub fn security_attributes(&mut self, attrs: c::LPSECURITY_ATTRIBUTES) { - self.security_attributes = attrs; + self.security_attributes = attrs as usize; } fn get_access_mode(&self) -> io::Result {