diff --git a/src/base.rs b/src/base.rs index 6ed9260..98169b1 100644 --- a/src/base.rs +++ b/src/base.rs @@ -389,11 +389,36 @@ pub fn get_effective_groupname() -> Option { } +/// An iterator over every user present on the system. +/// +/// This struct actually requires no fields, but has one hidden one to make it +/// `unsafe` to create. pub struct AllUsers(()); impl AllUsers { + + /// Creates a new iterator over every user present on the system. + /// + /// ## Unsafety + /// + /// This constructor is marked as `unsafe`, which is odd for a crate + /// that's meant to be a safe interface. It *has* to be unsafe because + /// we cannot guarantee that the underlying C functions, + /// `getpwent`/`setpwent`/`endpwent` that iterate over the system's + /// `passwd` entries, are called in a thread-safe manner. + /// + /// These functions [modify a global + /// state](http://man7.org/linux/man-pages/man3/getpwent.3.html# + /// ATTRIBUTES), and if any are used at the same time, the state could + /// be reset, resulting in a data race. We cannot even place it behind + /// an internal `Mutex`, as there is nothing stopping another `extern` + /// function definition from calling it! + /// + /// So to iterate all users, construct the iterator inside an `unsafe` + /// block, then make sure to not make a new instance of it until + /// iteration is over. pub unsafe fn new() -> AllUsers { - unsafe { setpwent() }; + setpwent(); AllUsers(()) } } diff --git a/src/cache.rs b/src/cache.rs index 74dadbd..ad35449 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -120,15 +120,19 @@ impl Default for UsersCache { impl UsersCache { - /// Create a new empty cache. + /// Creates a new empty cache. pub fn new() -> UsersCache { UsersCache::default() } + /// Creates a new cache that contains all the users present on the system. + /// + /// This is `unsafe` because we cannot prevent data races if two caches + /// were attempted to be initialised on different threads at the same time. pub unsafe fn with_all_users() -> UsersCache { let cache = UsersCache::new(); - for user in unsafe { AllUsers::new() } { + for user in AllUsers::new() { let uid = user.uid(); let user_arc = Arc::new(user); cache.users.forward.borrow_mut().insert(uid, Some(user_arc.clone()));