Skip to content

Commit

Permalink
More feedback and tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexcrichton
alexcrichton committed Apr 7, 2015
1 parent b3d1ad6 commit 91c0df5
Showing 1 changed file with 103 additions and 37 deletions.
140 changes: 103 additions & 37 deletions text/0000-io-fs-2.1.md
View file
Expand Up @@ -43,6 +43,12 @@ areas:

# Detailed design

First, a vision for how lowering APIs in general will be presented, and then a
number of specific APIs will each be proposed. Many of the proposed APIs are
independent from one another and this RFC may not be implemented all-in-one-go
but instead piecemeal over time, allowing the designs to evolve slightly in the
meantime.

## Lowering APIs

### The vision for the `os` module
Expand Down Expand Up @@ -103,7 +109,7 @@ crates in this regard.
Concretely, lowering has two ingredients:

1. Introducing one or more "raw" types that are generally direct aliases for C
types.
types (more on this in the next section).

2. Providing an extension trait that makes it possible to extract a raw type
from a `std` type. In some cases, it's possible to go the other way around as
Expand All @@ -120,6 +126,35 @@ lowerings that are currently missing from `std::fs`.
[io-reform-vision]: https://github.com/rust-lang/rfcs/blob/master/text/0517-io-os-reform.md#vision-for-io
[AsRawFd]: http://static.rust-lang.org/doc/master/std/os/unix/io/trait.AsRawFd.html

#### `std::os::platform::raw`

Each of the primitives in the standard library will expose the ability to be
lowered into its component abstraction, facilitating the need to define these
abstractions and organize them in the platform-specific modules. This RFC
proposes the following guidelines for doing so:

* Each platform will have a `raw` module inside of `std::os` which houses all of
its platform specific definitions.
* Only type definitions will be contained in `raw` modules, no function
bindings, methods, or trait implementations.
* Cross-platform types (e.g. those shared on all `unix` platforms) will be
located in the respective cross-platform module. Types which only differ in
the width of an integer type are considered to be cross-platform.
* Platform-specific types will exist only in the `raw` module for that platform.
A platform-specific type may have different field names, components, or just
not exist on other platforms.

Differences in integer widths are not considered to be enough of a platform
difference to define in each separate platform's module, meaning that it will be
possible to write code that uses `os::unix` but doesn't compile on all Unix
platforms. It is believed that most consumers of these types will continue to
store the same type (e.g. not assume it's an `i32`) throughout the application
or immediately cast it to a known type.

To reiterate, it is not planned for each `raw` module to provide *exhaustive*
bindings to each platform. Only those abstractions which the standard library is
lowering into will be defined in each `raw` module.

### Lowering `Metadata` (all platforms)

Currently the `Metadata` structure exposes very few pieces of information about
Expand All @@ -146,23 +181,23 @@ mod os::windows::fs {

mod os::unix::fs {
pub trait MetadataExt {
fn as_raw(&self) -> &raw::Metadata;
fn as_raw(&self) -> &Metadata;
}
impl MetadataExt for fs::Metadata { ... }

pub struct RawMetadata(libc::stat);
impl RawMetadata {
// Accessors for fields available in `libc::stat` for *all* platforms
fn dev(&self) -> libc::dev_t; // st_dev field
fn ino(&self) -> libc::ino_t; // st_ino field
fn raw_mode(&self) -> libc::mode_t; // st_mode field
fn nlink(&self) -> libc::nlink_t; // st_nlink field
fn uid(&self) -> libc::uid_t; // st_uid field
fn gid(&self) -> libc::gid_t; // st_gid field
fn rdev(&self) -> libc::dev_t; // st_rdev field
fn size(&self) -> libc::off_t; // st_size field
fn blksize(&self) -> libc::blksize_t; // st_blksize field
fn blocks(&self) -> libc::blkcnt_t; // st_blocks field
pub struct Metadata(raw::stat);
impl Metadata {
// Accessors for fields available in `raw::stat` for *all* platforms
fn dev(&self) -> raw::dev_t; // st_dev field
fn ino(&self) -> raw::ino_t; // st_ino field
fn mode(&self) -> raw::mode_t; // st_mode field
fn nlink(&self) -> raw::nlink_t; // st_nlink field
fn uid(&self) -> raw::uid_t; // st_uid field
fn gid(&self) -> raw::gid_t; // st_gid field
fn rdev(&self) -> raw::dev_t; // st_rdev field
fn size(&self) -> raw::off_t; // st_size field
fn blksize(&self) -> raw::blksize_t; // st_blksize field
fn blocks(&self) -> raw::blkcnt_t; // st_blocks field
fn atime(&self) -> (i64, i32); // st_atime field, (sec, nsec)
fn mtime(&self) -> (i64, i32); // st_mtime field, (sec, nsec)
fn ctime(&self) -> (i64, i32); // st_ctime field, (sec, nsec)
Expand All @@ -171,9 +206,16 @@ mod os::unix::fs {

// st_flags, st_gen, st_lspare, st_birthtim, st_qspare
mod os::{linux, macos, freebsd, ...}::fs {
pub struct stat { /* same public fields as libc::stat */ }
pub mod raw {
pub type dev_t = ...;
pub type ino_t = ...;
// ...
pub struct stat {
// ... same public fields as libc::stat
}
}
pub trait MetadataExt {
fn as_raw_stat(&self) -> &stat;
fn as_raw_stat(&self) -> &raw::stat;
}
impl MetadataExt for os::unix::fs::RawMetadata { ... }
impl MetadataExt for fs::Metadata { ... }
Expand Down Expand Up @@ -225,10 +267,31 @@ and it is unclear whether the current unstable methods of doing so,
piece of functionality is whether to provide a higher level abstractiong (e.g.
similar to the `bitflags` crate) for the permission bits on unix.

This RFC proposes renaming `mode` and `set_mode` on `PermissionsExt` and
`OpenOptionsExt` to `raw_mode` and `set_raw_mode` in order enable an addition of
a higher-level `Mode` abstraction in the future. This is also the rationale for
naming the accessor of `st_mode` on `RawMetadata` as `raw_mode`.
This RFC proposes considering the methods for stabilization as-is and not
pursuing a higher level abstraction of the unix permission bits. To facilitate
in their inspection and manipulation, however, the following constants will be
added:

```rust
mod os::unix::fs {
pub const USER_READ: raw::mode_t;
pub const USER_WRITE: raw::mode_t;
pub const USER_EXECUTE: raw::mode_t;
pub const USER_RWX: raw::mode_t;
pub const OTHER_READ: raw::mode_t;
pub const OTHER_WRITE: raw::mode_t;
pub const OTHER_EXECUTE: raw::mode_t;
pub const OTHER_RWX: raw::mode_t;
pub const GROUP_READ: raw::mode_t;
pub const GROUP_WRITE: raw::mode_t;
pub const GROUP_EXECUTE: raw::mode_t;
pub const GROUP_RWX: raw::mode_t;
pub const ALL_READ: raw::mode_t;
pub const ALL_WRITE: raw::mode_t;
pub const ALL_EXECUTE: raw::mode_t;
pub const ALL_RWX: raw::mode_t;
}

This comment has been minimized.

Sign in to view

Copy link
@lambda

lambda Apr 8, 2015

This is missing a couple of constants; the SUID, SGID, and sticky bit flags (S_ISUID, S_ISGID, and S_ISVTX in POSIX/Single Unix Specificaiton). I appreciate the more readable names for the other constants, so more readable constants for these would be good too, perhaps something like SETUID, SETGID (with or without an underscore, I don't particularly mind), and STICKY_BIT since that's how people usually refer to them.

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Apr 8, 2015

Author Owner

Oops thanks for the reminder! I haven't worked with the sticky bit much before, so I'm wondering if the name STICKY would be appropriate? I figure the "bit" is implied as these are all just bit twiddling anyway.

This comment has been minimized.

Sign in to view

Copy link
@lambda

lambda Apr 8, 2015

Just STICKY would probably be unambiguous, but it sounds a little odd. It's almost always referred to as the "sticky bit", even when the others aren't. man 2 chmod on Mac OS X:

 The ISVTX (the sticky bit) indicates to the system which executable files
 are shareable (the default) and the system maintains the program text of
 the files in the swap area. The sticky bit may only be set by the super
 user on shareable executable files.

 If mode ISVTX (the `sticky bit') is set on a directory, an unprivileged
 user may not delete or rename files of other users in that directory. The
 sticky bit may be set by any user on a directory which the user owns or
 has appropriate permissions.  For more details of the properties of the
 sticky bit, see sticky(8).

Though man sticky does use "sticky" alone in at least one case:

 A directory whose `sticky bit' is set becomes an append-only directory,
 or, more accurately, a directory in which the deletion of files is
 restricted.  A file in a sticky directory may only be removed or renamed
 by a user if the user has write permission for the directory and the user
 is the owner of the file, the owner of the directory, or the super-user.
 This feature is usefully applied to directories such as /tmp which must
 be publicly writable but should deny users the license to arbitrarily
 delete or rename each others' files.

 Any user may create a sticky directory.  See chmod(1) for details about
 modifying file modes.

man 2 chmod on Linux:

   S_ISVTX  (01000)  sticky bit (restricted deletion flag, as described
                     in unlink(2))

man unlink on Linux:

   EPERM or EACCES
          The directory containing pathname has the sticky bit (S_ISVTX)
          set and the process's effective UID is neither the UID of the
          file to be deleted nor that of the directory containing it,
          and the process is not privileged (Linux: does not have the
          CAP_FOWNER capability).

So, for whatever reason, it's almost always referred to as "the sticky bit" even when none of the others contain "bit". I don't feel particularly strongly either way, I'd be fine with just STICKY, I just figured I'd point out some usage examples. The standard name, S_ISVTX, is completely opaque, and the traditional name "save swapped text" is entirely obsolete, so STICKY or STICKY_BIT are pretty much the only reasonable options.

As for its use, it's usually used in shared directories, which are writable by multiple users, but in which you don't want one user to accidentally delete another user's files; it prevents users from deleting files belonging to other users, even though they have write access to the directory.

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Apr 8, 2015

Author Owner

Alright it definitely sounds like the conventional way to refer to this is include "bit" in the name, so I'll do that. Thanks for the info!
```

Finally, the `set_permissions` function of the `std::fs` module is also proposed
to be marked `#[stable]` soon as a method of blanket setting permissions for a
Expand All @@ -245,17 +308,11 @@ impl Permissions {
///
/// On unix platforms this corresponds to the permission bits `0o666`
pub fn new() -> Permissions;

/// Creates a new set of permissions which when applied to a file will make
/// it read-only for all users.
///
/// On unix platforms this corresponds to the permission bits `0o444`.
pub fn new_readonly() -> Permissions;
}

mod os::unix::fs {
pub trait PermissionsExt {
fn from_raw_mode(mode: i32) -> Self;
fn from_mode(mode: raw::mode_t) -> Self;
}
impl PermissionsExt for Permissions { ... }
}
Expand All @@ -280,22 +337,14 @@ impl CreateDirOptions {
/// permissions settings.
pub fn recursive(&mut self, recursive: bool) -> &mut Self;

/// Use the specified directory as a "template" for permissions and security
/// settings of the new directories to be created.
///
/// On unix this will issue a `stat` of the specified directory and new
/// directories will be created with the same permission bits. On Windows
/// this will trigger the use of the `CreateDirectoryEx` function.
pub fn template<P: AsRef<Path>>(&mut self, path: P) -> &mut Self;

/// Create the specified directory with the options configured in this
/// builder.
pub fn create<P: AsRef<Path>>(&self, path: P) -> io::Result<()>;
}

mod os::unix::fs {
pub trait CreateDirOptionsExt {
fn raw_mode(&mut self, mode: i32) -> &mut Self;
fn mode(&mut self, mode: raw::mode_t) -> &mut Self;
}
impl CreateDirOptionsExt for CreateDirOptions { ... }
}
Expand All @@ -307,9 +356,26 @@ mod os::windows::fs {
}
impl CreateDirOptionsExt for CreateDirOptions { ... }
}
```

This sort of builder is also extendable to other flavors of functions in the
future, such as [C++'s template parameter][cpp-dir-template]:

[cpp-dir-template]: http://en.cppreference.com/w/cpp/experimental/fs/create_directory

```rust
/// Use the specified directory as a "template" for permissions and security
/// settings of the new directories to be created.
///
/// On unix this will issue a `stat` of the specified directory and new
/// directories will be created with the same permission bits. On Windows
/// this will trigger the use of the `CreateDirectoryEx` function.
pub fn template<P: AsRef<Path>>(&mut self, path: P) -> &mut Self;
```

At this time, however, it it not proposed to add this method to
`CreateDirOptions`.

## Adding `fs::equivalent`

A new function `equivalent` will be added to the `fs` module along the lines of
Expand Down

0 comments on commit 91c0df5

Please sign in to comment.