From 83ddcc886c03345b4405b183acbd8491f08afdc4 Mon Sep 17 00:00:00 2001
From: Benjamin Sago <ogham@bsago.me>
Date: Tue, 22 Dec 2015 20:07:05 +1100
Subject: [PATCH 1/7] io::Error -> IOError, io::Result -> IOResult

---
 src/lib.rs | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 426ac39..61f6a0b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -113,7 +113,7 @@ use std::borrow::ToOwned;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::collections::HashMap;
 use std::ffi::{CStr, CString};
-use std::io;
+use std::io::{Error as IOError, Result as IOResult};
 use std::ptr::read;
 use std::str::from_utf8_unchecked;
 
@@ -578,55 +578,55 @@ pub fn get_effective_groupname() -> Option<String> {
 }
 
 /// Set current user for the running process, requires root priviledges.
-pub fn set_current_uid(uid: uid_t) -> Result<(), io::Error> {
+pub fn set_current_uid(uid: uid_t) -> IOResult<()> {
     match unsafe { setuid(uid) } {
         0 => Ok(()),
-        -1 => Err(io::Error::last_os_error()),
+        -1 => Err(IOError::last_os_error()),
         _ => unreachable!()
     }
 }
 
 /// Set current group for the running process, requires root priviledges.
-pub fn set_current_gid(gid: gid_t) -> Result<(), io::Error> {
+pub fn set_current_gid(gid: gid_t) -> IOResult<()> {
     match unsafe { setgid(gid) } {
         0 => Ok(()),
-        -1 => Err(io::Error::last_os_error()),
+        -1 => Err(IOError::last_os_error()),
         _ => unreachable!()
     }
 }
 
 /// Set effective user for the running process, requires root priviledges.
-pub fn set_effective_uid(uid: uid_t) -> Result<(), io::Error> {
+pub fn set_effective_uid(uid: uid_t) -> IOResult<()> {
     match unsafe { seteuid(uid) } {
         0 => Ok(()),
-        -1 => Err(io::Error::last_os_error()),
+        -1 => Err(IOError::last_os_error()),
         _ => unreachable!()
     }
 }
 
 /// Set effective user for the running process, requires root priviledges.
-pub fn set_effective_gid(gid: gid_t) -> Result<(), io::Error> {
+pub fn set_effective_gid(gid: gid_t) -> IOResult<()> {
     match unsafe { setegid(gid) } {
         0 => Ok(()),
-        -1 => Err(io::Error::last_os_error()),
+        -1 => Err(IOError::last_os_error()),
         _ => unreachable!()
     }
 }
 
 /// Atomically set current and effective user for the running process, requires root priviledges.
-pub fn set_both_uid(ruid: uid_t, euid: uid_t) -> Result<(), io::Error> {
+pub fn set_both_uid(ruid: uid_t, euid: uid_t) -> IOResult<()> {
     match unsafe { setreuid(ruid, euid) } {
         0 => Ok(()),
-        -1 => Err(io::Error::last_os_error()),
+        -1 => Err(IOError::last_os_error()),
         _ => unreachable!()
     }
 }
 
 /// Atomically set current and effective group for the running process, requires root priviledges.
-pub fn set_both_gid(rgid: gid_t, egid: gid_t) -> Result<(), io::Error> {
+pub fn set_both_gid(rgid: gid_t, egid: gid_t) -> IOResult<()> {
     match unsafe { setregid(rgid, egid) } {
         0 => Ok(()),
-        -1 => Err(io::Error::last_os_error()),
+        -1 => Err(IOError::last_os_error()),
         _ => unreachable!()
     }
 }
@@ -659,7 +659,7 @@ impl Drop for SwitchUserGuard {
 /// Use with care! Possible security issues can happen, as Rust doesn't
 /// guarantee running the destructor! If in doubt run `drop()` method
 /// on the guard value manually!
-pub fn switch_user_group(uid: uid_t, gid: gid_t) -> Result<SwitchUserGuard, io::Error> {
+pub fn switch_user_group(uid: uid_t, gid: gid_t) -> Result<SwitchUserGuard, IOError> {
     let current_state = SwitchUserGuard {
         uid: get_effective_uid(),
         gid: get_effective_gid(),

From bb1299a2884152bc553729d55d625010c1ac7de3 Mon Sep 17 00:00:00 2001
From: Benjamin Sago <ogham@bsago.me>
Date: Sun, 10 Jan 2016 21:14:05 +1300
Subject: [PATCH 2/7] Extract OS functions to os module

This commit does two things:

Firstly, it swaps the locations in the code where the actual, unsafe C library operations are done from being done in the OSUsers struct to in the stand-alone functions. This makes more sense, as previously, the stand-alone functions would instantiate a new cache, run a method on it, and then immediately discard the cache. Now the cache uses the methods, not the other way round.

Secondly, it extracts all of the caching methods to its own module, leaving `lib` with a bare-bones users implementation.
---
 src/lib.rs | 303 ++++++++++-------------------------------------------
 src/os.rs  | 195 ++++++++++++++++++++++++++++++++++
 2 files changed, 251 insertions(+), 247 deletions(-)
 create mode 100644 src/os.rs

diff --git a/src/lib.rs b/src/lib.rs
index 61f6a0b..5dfccf0 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -108,10 +108,6 @@
 //! Use the mocking module to create custom tables to test your code for these
 //! edge cases.
 
-
-use std::borrow::ToOwned;
-use std::collections::hash_map::Entry::{Occupied, Vacant};
-use std::collections::HashMap;
 use std::ffi::{CStr, CString};
 use std::io::{Error as IOError, Result as IOResult};
 use std::ptr::read;
@@ -127,7 +123,8 @@ use libc::{c_char, time_t};
 use libc::c_char;
 
 pub mod mock;
-
+pub mod os;
+pub use os::OSUsers;
 
 /// The trait for the `OSUsers` object.
 pub trait Users {
@@ -261,21 +258,6 @@ pub struct Group {
     pub members: Vec<String>,
 }
 
-/// A producer of user and group instances that caches every result.
-#[derive(Clone)]
-pub struct OSUsers {
-    users: HashMap<uid_t, Option<User>>,
-    users_back: HashMap<String, Option<uid_t>>,
-
-    groups: HashMap<gid_t, Option<Group>>,
-    groups_back: HashMap<String, Option<gid_t>>,
-
-    uid: Option<uid_t>,
-    gid: Option<gid_t>,
-    euid: Option<uid_t>,
-    egid: Option<gid_t>,
-}
-
 unsafe fn from_raw_buf(p: *const i8) -> String {
     from_utf8_unchecked(CStr::from_ptr(p).to_bytes()).to_string()
 }
@@ -328,253 +310,85 @@ unsafe fn members(groups: *const *const c_char) -> Vec<String> {
     }
 }
 
-impl Users for OSUsers {
-    fn get_user_by_uid(&mut self, uid: uid_t) -> Option<User> {
-        match self.users.entry(uid) {
-            Vacant(entry) => {
-                let user = unsafe { passwd_to_user(getpwuid(uid)) };
-                match user {
-                    Some(user) => {
-                        entry.insert(Some(user.clone()));
-                        self.users_back.insert(user.name.clone(), Some(user.uid));
-                        Some(user)
-                    },
-                    None => {
-                        entry.insert(None);
-                        None
-                    }
-                }
-            },
-            Occupied(entry) => entry.get().clone(),
-        }
-    }
-
-    fn get_user_by_name(&mut self, username: &str) -> Option<User> {
-        // to_owned() could change here:
-        // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
-        match self.users_back.entry(username.to_owned()) {
-            Vacant(entry) => {
-                let username_c = CString::new(username);
-
-                if !username_c.is_ok() {
-                    // This usually means the given username contained a '\0' already
-                    // It is debatable what to do here
-                    return None;
-                }
-
-                let user = unsafe { passwd_to_user(getpwnam(username_c.unwrap().as_ptr())) };
-                match user {
-                    Some(user) => {
-                        entry.insert(Some(user.uid));
-                        self.users.insert(user.uid, Some(user.clone()));
-                        Some(user)
-                    },
-                    None => {
-                        entry.insert(None);
-                        None
-                    }
-                }
-            },
-            Occupied(entry) => match entry.get() {
-                &Some(uid) => self.users[&uid].clone(),
-                &None => None,
-            }
-        }
-    }
-
-    fn get_group_by_gid(&mut self, gid: gid_t) -> Option<Group> {
-        match self.groups.entry(gid) {
-            Vacant(entry) => {
-                let group = unsafe { struct_to_group(getgrgid(gid)) };
-                match group {
-                    Some(group) => {
-                        entry.insert(Some(group.clone()));
-                        self.groups_back.insert(group.name.clone(), Some(group.gid));
-                        Some(group)
-                    },
-                    None => {
-                        entry.insert(None);
-                        None
-                    }
-                }
-            },
-            Occupied(entry) => entry.get().clone(),
-        }
-    }
-
-    fn get_group_by_name(&mut self, group_name: &str) -> Option<Group> {
-        // to_owned() could change here:
-        // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
-        match self.groups_back.entry(group_name.to_owned()) {
-            Vacant(entry) => {
-                let group_name_c = CString::new(group_name);
-
-                if !group_name_c.is_ok() {
-                    // This usually means the given username contained a '\0' already
-                    // It is debatable what to do here
-                    return None;
-                }
-
-                let user = unsafe { struct_to_group(getgrnam(group_name_c.unwrap().as_ptr())) };
-                match user {
-                    Some(group) => {
-                        entry.insert(Some(group.gid));
-                        self.groups.insert(group.gid, Some(group.clone()));
-                        Some(group)
-                    },
-                    None => {
-                        entry.insert(None);
-                        None
-                    }
-                }
-            },
-            Occupied(entry) => match entry.get() {
-                &Some(gid) => self.groups[&gid].clone(),
-                &None => None,
-            }
-        }
-    }
-
-    fn get_current_uid(&mut self) -> uid_t {
-        match self.uid {
-            Some(uid) => uid,
-            None => {
-                let uid = unsafe { getuid() };
-                self.uid = Some(uid);
-                uid
-            }
-        }
-    }
-
-    /// Return the username of the user running the process.
-    fn get_current_username(&mut self) -> Option<String> {
-        let uid = self.get_current_uid();
-        self.get_user_by_uid(uid).map(|u| u.name)
-    }
-
-    fn get_current_gid(&mut self) -> gid_t {
-        match self.gid {
-            Some(gid) => gid,
-            None => {
-                let gid = unsafe { getgid() };
-                self.gid = Some(gid);
-                gid
-            }
-        }
-    }
-
-    fn get_current_groupname(&mut self) -> Option<String> {
-        let gid = self.get_current_gid();
-        self.get_group_by_gid(gid).map(|g| g.name)
-    }
-
-    fn get_effective_gid(&mut self) -> gid_t {
-        match self.egid {
-            Some(gid) => gid,
-            None => {
-                let gid = unsafe { getegid() };
-                self.egid = Some(gid);
-                gid
-            }
-        }
-    }
-
-    fn get_effective_groupname(&mut self) -> Option<String> {
-        let gid = self.get_effective_gid();
-        self.get_group_by_gid(gid).map(|g| g.name)
-    }
-
-    fn get_effective_uid(&mut self) -> uid_t {
-        match self.euid {
-            Some(uid) => uid,
-            None => {
-                let uid = unsafe { geteuid() };
-                self.euid = Some(uid);
-                uid
-            }
-        }
-    }
-
-    fn get_effective_username(&mut self) -> Option<String> {
-        let uid = self.get_effective_uid();
-        self.get_user_by_uid(uid).map(|u| u.name)
-    }
-}
-
-impl OSUsers {
-    /// Create a new empty OS Users object.
-    pub fn empty_cache() -> OSUsers {
-        OSUsers {
-            users:       HashMap::new(),
-            users_back:  HashMap::new(),
-            groups:      HashMap::new(),
-            groups_back: HashMap::new(),
-            uid:         None,
-            gid:         None,
-            euid:        None,
-            egid:        None,
-        }
-    }
-}
 
 /// Return a User object if one exists for the given user ID; otherwise, return None.
 pub fn get_user_by_uid(uid: uid_t) -> Option<User> {
-    OSUsers::empty_cache().get_user_by_uid(uid)
+    unsafe { passwd_to_user(getpwuid(uid)) }
 }
 
 /// Return a User object if one exists for the given username; otherwise, return None.
 pub fn get_user_by_name(username: &str) -> Option<User> {
-    OSUsers::empty_cache().get_user_by_name(username)
+    let username_c = CString::new(username);
+
+    if !username_c.is_ok() {
+        // This usually means the given username contained a '\0' already
+        // It is debatable what to do here
+        return None;
+    }
+
+    unsafe { passwd_to_user(getpwnam(username_c.unwrap().as_ptr())) }
 }
 
 /// Return a Group object if one exists for the given group ID; otherwise, return None.
 pub fn get_group_by_gid(gid: gid_t) -> Option<Group> {
-    OSUsers::empty_cache().get_group_by_gid(gid)
+    unsafe { struct_to_group(getgrgid(gid)) }
 }
 
 /// Return a Group object if one exists for the given groupname; otherwise, return None.
 pub fn get_group_by_name(group_name: &str) -> Option<Group> {
-    OSUsers::empty_cache().get_group_by_name(group_name)
+    let group_name_c = CString::new(group_name);
+
+    if !group_name_c.is_ok() {
+        // This usually means the given username contained a '\0' already
+        // It is debatable what to do here
+        return None;
+    }
+
+    unsafe { struct_to_group(getgrnam(group_name_c.unwrap().as_ptr())) }
 }
 
 /// Return the user ID for the user running the process.
 pub fn get_current_uid() -> uid_t {
-    OSUsers::empty_cache().get_current_uid()
+    unsafe { getuid() }
 }
 
 /// Return the username of the user running the process.
 pub fn get_current_username() -> Option<String> {
-    OSUsers::empty_cache().get_current_username()
+    let uid = get_current_uid();
+    get_user_by_uid(uid).map(|u| u.name)
 }
 
 /// Return the user ID for the effective user running the process.
 pub fn get_effective_uid() -> uid_t {
-    OSUsers::empty_cache().get_effective_uid()
+    unsafe { geteuid() }
 }
 
 /// Return the username of the effective user running the process.
 pub fn get_effective_username() -> Option<String> {
-    OSUsers::empty_cache().get_effective_username()
+    let uid = get_effective_uid();
+    get_user_by_uid(uid).map(|u| u.name)
 }
 
 /// Return the group ID for the user running the process.
 pub fn get_current_gid() -> gid_t {
-    OSUsers::empty_cache().get_current_gid()
+    unsafe { getgid() }
 }
 
 /// Return the groupname of the user running the process.
 pub fn get_current_groupname() -> Option<String> {
-    OSUsers::empty_cache().get_current_groupname()
+    let gid = get_current_gid();
+    get_group_by_gid(gid).map(|g| g.name)
 }
 
 /// Return the group ID for the effective user running the process.
 pub fn get_effective_gid() -> gid_t {
-    OSUsers::empty_cache().get_effective_gid()
+    unsafe { getegid() }
 }
 
 /// Return the groupname of the effective user running the process.
 pub fn get_effective_groupname() -> Option<String> {
-    OSUsers::empty_cache().get_effective_groupname()
+    let gid = get_effective_gid();
+    get_group_by_gid(gid).map(|g| g.name)
 }
 
 /// Set current user for the running process, requires root priviledges.
@@ -670,79 +484,74 @@ pub fn switch_user_group(uid: uid_t, gid: gid_t) -> Result<SwitchUserGuard, IOEr
     Ok(current_state)
 }
 
+
 #[cfg(test)]
 mod test {
-    use super::{Users, OSUsers, get_current_username};
+    use super::*;
 
     #[test]
     fn uid() {
-        OSUsers::empty_cache().get_current_uid();
+        get_current_uid();
     }
 
     #[test]
     fn username() {
-        let mut users = OSUsers::empty_cache();
-        let uid = users.get_current_uid();
-        assert_eq!(get_current_username().unwrap(), users.get_user_by_uid(uid).unwrap().name);
+        let uid = get_current_uid();
+        assert_eq!(get_current_username().unwrap(), get_user_by_uid(uid).unwrap().name);
     }
 
     #[test]
     fn uid_for_username() {
-        let mut users = OSUsers::empty_cache();
-        let uid = users.get_current_uid();
-        let user = users.get_user_by_uid(uid).unwrap();
+        let uid = get_current_uid();
+        let user = get_user_by_uid(uid).unwrap();
         assert_eq!(user.uid, uid);
     }
 
     #[test]
     fn username_for_uid_for_username() {
-        let mut users = OSUsers::empty_cache();
-        let uid = users.get_current_uid();
-        let user = users.get_user_by_uid(uid).unwrap();
-        let user2 = users.get_user_by_uid(user.uid).unwrap();
+        let uid = get_current_uid();
+        let user = get_user_by_uid(uid).unwrap();
+        let user2 = get_user_by_uid(user.uid).unwrap();
         assert_eq!(user2.uid, uid);
     }
 
     #[test]
     fn user_info() {
-        let mut users = OSUsers::empty_cache();
-        let uid = users.get_current_uid();
-        let user = users.get_user_by_uid(uid).unwrap();
+        let uid = get_current_uid();
+        let user = get_user_by_uid(uid).unwrap();
         // Not a real test but can be used to verify correct results
         // Use with --nocapture on test executable to show output
         println!("HOME={}, SHELL={}", user.home_dir, user.shell);
     }
 
     #[test]
-    fn get_user_by_name() {
+    fn user_by_name() {
         // We cannot really test for arbitrary user as they might not exist on the machine
         // Instead the name of the current user is used
-        let mut users = OSUsers::empty_cache();
-        let name = users.get_current_username().unwrap();
-        let user_by_name = users.get_user_by_name(&name);
+        let name = get_current_username().unwrap();
+        let user_by_name = get_user_by_name(&name);
         assert!(user_by_name.is_some());
         assert_eq!(user_by_name.unwrap().name, name);
 
         // User names containing '\0' cannot be used (for now)
-        let user = users.get_user_by_name("user\0");
+        let user = get_user_by_name("user\0");
         assert!(user.is_none());
     }
 
     #[test]
-    fn get_group_by_name() {
+    fn group_by_name() {
         // We cannot really test for arbitrary groups as they might not exist on the machine
         // Instead the primary group of the current user is used
-        let mut users = OSUsers::empty_cache();
-        let cur_uid = users.get_current_uid();
-        let cur_user = users.get_user_by_uid(cur_uid).unwrap();
-        let cur_group = users.get_group_by_gid(cur_user.primary_group).unwrap();
-        let group_by_name = users.get_group_by_name(&cur_group.name);
+        let cur_uid = get_current_uid();
+        let cur_user = get_user_by_uid(cur_uid).unwrap();
+        let cur_group = get_group_by_gid(cur_user.primary_group).unwrap();
+        let group_by_name = get_group_by_name(&cur_group.name);
 
         assert!(group_by_name.is_some());
         assert_eq!(group_by_name.unwrap().name, cur_group.name);
 
         // Group names containing '\0' cannot be used (for now)
-        let group = users.get_group_by_name("users\0");
+        let group = get_group_by_name("users\0");
         assert!(group.is_none());
     }
 }
diff --git a/src/os.rs b/src/os.rs
new file mode 100644
index 0000000..6298a2a
--- /dev/null
+++ b/src/os.rs
@@ -0,0 +1,195 @@
+use libc::{uid_t, gid_t};
+use std::borrow::ToOwned;
+use std::collections::hash_map::Entry::{Occupied, Vacant};
+use std::collections::HashMap;
+
+use super::{User, Group, Users};
+
+
+/// A producer of user and group instances that caches every result.
+#[derive(Clone)]
+pub struct OSUsers {
+    users: HashMap<uid_t, Option<User>>,
+    users_back: HashMap<String, Option<uid_t>>,
+
+    groups: HashMap<gid_t, Option<Group>>,
+    groups_back: HashMap<String, Option<gid_t>>,
+
+    uid: Option<uid_t>,
+    gid: Option<gid_t>,
+    euid: Option<uid_t>,
+    egid: Option<gid_t>,
+}
+
+impl OSUsers {
+    /// Create a new empty OS Users object.
+    pub fn empty_cache() -> OSUsers {
+        OSUsers {
+            users:       HashMap::new(),
+            users_back:  HashMap::new(),
+            groups:      HashMap::new(),
+            groups_back: HashMap::new(),
+            uid:         None,
+            gid:         None,
+            euid:        None,
+            egid:        None,
+        }
+    }
+}
+
+impl Users for OSUsers {
+    fn get_user_by_uid(&mut self, uid: uid_t) -> Option<User> {
+        match self.users.entry(uid) {
+            Vacant(entry) => {
+                let user = super::get_user_by_uid(uid);
+                match user {
+                    Some(user) => {
+                        entry.insert(Some(user.clone()));
+                        self.users_back.insert(user.name.clone(), Some(user.uid));
+                        Some(user)
+                    },
+                    None => {
+                        entry.insert(None);
+                        None
+                    }
+                }
+            },
+            Occupied(entry) => entry.get().clone(),
+        }
+    }
+
+    fn get_user_by_name(&mut self, username: &str) -> Option<User> {
+        // to_owned() could change here:
+        // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
+        match self.users_back.entry(username.to_owned()) {
+            Vacant(entry) => {
+                let user = super::get_user_by_name(username);
+                match user {
+                    Some(user) => {
+                        entry.insert(Some(user.uid));
+                        self.users.insert(user.uid, Some(user.clone()));
+                        Some(user)
+                    },
+                    None => {
+                        entry.insert(None);
+                        None
+                    }
+                }
+            },
+            Occupied(entry) => match entry.get() {
+                &Some(uid) => self.users[&uid].clone(),
+                &None => None,
+            }
+        }
+    }
+
+    fn get_group_by_gid(&mut self, gid: gid_t) -> Option<Group> {
+        match self.groups.entry(gid) {
+            Vacant(entry) => {
+                let group = super::get_group_by_gid(gid);
+                match group {
+                    Some(group) => {
+                        entry.insert(Some(group.clone()));
+                        self.groups_back.insert(group.name.clone(), Some(group.gid));
+                        Some(group)
+                    },
+                    None => {
+                        entry.insert(None);
+                        None
+                    }
+                }
+            },
+            Occupied(entry) => entry.get().clone(),
+        }
+    }
+
+    fn get_group_by_name(&mut self, group_name: &str) -> Option<Group> {
+        // to_owned() could change here:
+        // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
+        match self.groups_back.entry(group_name.to_owned()) {
+            Vacant(entry) => {
+                let user = super::get_group_by_name(group_name);
+                match user {
+                    Some(group) => {
+                        entry.insert(Some(group.gid));
+                        self.groups.insert(group.gid, Some(group.clone()));
+                        Some(group)
+                    },
+                    None => {
+                        entry.insert(None);
+                        None
+                    }
+                }
+            },
+            Occupied(entry) => match entry.get() {
+                &Some(gid) => self.groups[&gid].clone(),
+                &None => None,
+            }
+        }
+    }
+
+    fn get_current_uid(&mut self) -> uid_t {
+        match self.uid {
+            Some(uid) => uid,
+            None => {
+                let uid = super::get_current_uid();
+                self.uid = Some(uid);
+                uid
+            }
+        }
+    }
+
+    /// Return the username of the user running the process.
+    fn get_current_username(&mut self) -> Option<String> {
+        let uid = self.get_current_uid();
+        self.get_user_by_uid(uid).map(|u| u.name)
+    }
+
+    fn get_current_gid(&mut self) -> gid_t {
+        match self.gid {
+            Some(gid) => gid,
+            None => {
+                let gid = super::get_current_gid();
+                self.gid = Some(gid);
+                gid
+            }
+        }
+    }
+
+    fn get_current_groupname(&mut self) -> Option<String> {
+        let gid = self.get_current_gid();
+        self.get_group_by_gid(gid).map(|g| g.name)
+    }
+
+    fn get_effective_gid(&mut self) -> gid_t {
+        match self.egid {
+            Some(gid) => gid,
+            None => {
+                let gid = super::get_effective_gid();
+                self.egid = Some(gid);
+                gid
+            }
+        }
+    }
+
+    fn get_effective_groupname(&mut self) -> Option<String> {
+        let gid = self.get_effective_gid();
+        self.get_group_by_gid(gid).map(|g| g.name)
+    }
+
+    fn get_effective_uid(&mut self) -> uid_t {
+        match self.euid {
+            Some(uid) => uid,
+            None => {
+                let uid = super::get_effective_uid();
+                self.euid = Some(uid);
+                uid
+            }
+        }
+    }
+
+    fn get_effective_username(&mut self) -> Option<String> {
+        let uid = self.get_effective_uid();
+        self.get_user_by_uid(uid).map(|u| u.name)
+    }
+}

From 04f52c4de926eca0e5da5e42e9928aa63fcae57c Mon Sep 17 00:00:00 2001
From: Benjamin Sago <ogham@bsago.me>
Date: Sun, 10 Jan 2016 21:15:50 +1300
Subject: [PATCH 3/7] Remove unnecessary 'pub's in C structs

These structures are all private to the current module anyway, so these `pub`s do nothing.
---
 src/lib.rs | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 5dfccf0..d38ff9d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -169,36 +169,36 @@ pub trait Users {
 #[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "dragonfly"))]
 #[repr(C)]
 struct c_passwd {
-    pub pw_name:    *const c_char,  // user name
-    pub pw_passwd:  *const c_char,  // password field
-    pub pw_uid:     uid_t,          // user ID
-    pub pw_gid:     gid_t,          // group ID
-    pub pw_change:  time_t,         // password change time
-    pub pw_class:   *const c_char,
-    pub pw_gecos:   *const c_char,
-    pub pw_dir:     *const c_char,  // user's home directory
-    pub pw_shell:   *const c_char,  // user's shell
-    pub pw_expire:  time_t,         // password expiry time
+    pw_name:    *const c_char,  // user name
+    pw_passwd:  *const c_char,  // password field
+    pw_uid:     uid_t,          // user ID
+    pw_gid:     gid_t,          // group ID
+    pw_change:  time_t,         // password change time
+    pw_class:   *const c_char,
+    pw_gecos:   *const c_char,
+    pw_dir:     *const c_char,  // user's home directory
+    pw_shell:   *const c_char,  // user's shell
+    pw_expire:  time_t,         // password expiry time
 }
 
 #[cfg(target_os = "linux")]
 #[repr(C)]
 struct c_passwd {
-    pub pw_name:    *const c_char,  // user name
-    pub pw_passwd:  *const c_char,  // password field
-    pub pw_uid:     uid_t,          // user ID
-    pub pw_gid:     gid_t,          // group ID
-    pub pw_gecos:   *const c_char,
-    pub pw_dir:     *const c_char,  // user's home directory
-    pub pw_shell:   *const c_char,  // user's shell
+    pw_name:    *const c_char,  // user name
+    pw_passwd:  *const c_char,  // password field
+    pw_uid:     uid_t,          // user ID
+    pw_gid:     gid_t,          // group ID
+    pw_gecos:   *const c_char,
+    pw_dir:     *const c_char,  // user's home directory
+    pw_shell:   *const c_char,  // user's shell
 }
 
 #[repr(C)]
 struct c_group {
-    pub gr_name:   *const c_char,         // group name
-    pub gr_passwd: *const c_char,         // password
-    pub gr_gid:    gid_t,                 // group id
-    pub gr_mem:    *const *const c_char,  // names of users in the group
+    gr_name:   *const c_char,         // group name
+    gr_passwd: *const c_char,         // password
+    gr_gid:    gid_t,                 // group id
+    gr_mem:    *const *const c_char,  // names of users in the group
 }
 
 extern {

From 6a829c7b7db3e33a69cd77b8b46e5bb420c2becf Mon Sep 17 00:00:00 2001
From: Benjamin Sago <ogham@bsago.me>
Date: Wed, 13 Jan 2016 18:00:20 +1300
Subject: [PATCH 4/7] Extract bi-map groups into BiMap struct

The pairing of forward and backward maps was repeated twice: once for users, and once for groups. This commit refactors them into a BiMap struct that contains HashMaps of the relevant types.

Note that these new structs are just dumb structs: they don't implement any BiMap functionality, and all the other pieces of code don't do anything different with the maps -- they just refer to the struct's fields, making this a smaller change than it could be.
---
 src/os.rs | 52 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/src/os.rs b/src/os.rs
index 6298a2a..8cc6805 100644
--- a/src/os.rs
+++ b/src/os.rs
@@ -9,11 +9,8 @@ use super::{User, Group, Users};
 /// A producer of user and group instances that caches every result.
 #[derive(Clone)]
 pub struct OSUsers {
-    users: HashMap<uid_t, Option<User>>,
-    users_back: HashMap<String, Option<uid_t>>,
-
-    groups: HashMap<gid_t, Option<Group>>,
-    groups_back: HashMap<String, Option<gid_t>>,
+    users: BiMap<uid_t, User>,
+    groups: BiMap<gid_t, Group>,
 
     uid: Option<uid_t>,
     gid: Option<gid_t>,
@@ -21,14 +18,31 @@ pub struct OSUsers {
     egid: Option<gid_t>,
 }
 
+/// A kinda-bi-directional HashMap that associates keys to values, and then
+/// strings back to keys. It doesn’t go the full route and offer
+/// *values*-to-keys lookup, because we only want to search based on
+/// usernames and group names. There wouldn’t be much point offering a “User
+/// to uid” map, as the uid is present in the user struct!
+#[derive(Clone)]
+struct BiMap<K, V> {
+    forward:  HashMap<K, Option<V>>,
+    backward: HashMap<String, Option<K>>,
+}
+
 impl OSUsers {
     /// Create a new empty OS Users object.
     pub fn empty_cache() -> OSUsers {
         OSUsers {
-            users:       HashMap::new(),
-            users_back:  HashMap::new(),
-            groups:      HashMap::new(),
-            groups_back: HashMap::new(),
+            users: BiMap {
+                forward:  HashMap::new(),
+                backward: HashMap::new(),
+            },
+
+            groups: BiMap {
+                forward:  HashMap::new(),
+                backward: HashMap::new(),
+            },
+
             uid:         None,
             gid:         None,
             euid:        None,
@@ -39,13 +53,13 @@ impl OSUsers {
 
 impl Users for OSUsers {
     fn get_user_by_uid(&mut self, uid: uid_t) -> Option<User> {
-        match self.users.entry(uid) {
+        match self.users.forward.entry(uid) {
             Vacant(entry) => {
                 let user = super::get_user_by_uid(uid);
                 match user {
                     Some(user) => {
                         entry.insert(Some(user.clone()));
-                        self.users_back.insert(user.name.clone(), Some(user.uid));
+                        self.users.backward.insert(user.name.clone(), Some(user.uid));
                         Some(user)
                     },
                     None => {
@@ -61,13 +75,13 @@ impl Users for OSUsers {
     fn get_user_by_name(&mut self, username: &str) -> Option<User> {
         // to_owned() could change here:
         // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
-        match self.users_back.entry(username.to_owned()) {
+        match self.users.backward.entry(username.to_owned()) {
             Vacant(entry) => {
                 let user = super::get_user_by_name(username);
                 match user {
                     Some(user) => {
                         entry.insert(Some(user.uid));
-                        self.users.insert(user.uid, Some(user.clone()));
+                        self.users.forward.insert(user.uid, Some(user.clone()));
                         Some(user)
                     },
                     None => {
@@ -77,20 +91,20 @@ impl Users for OSUsers {
                 }
             },
             Occupied(entry) => match entry.get() {
-                &Some(uid) => self.users[&uid].clone(),
+                &Some(uid) => self.users.forward[&uid].clone(),
                 &None => None,
             }
         }
     }
 
     fn get_group_by_gid(&mut self, gid: gid_t) -> Option<Group> {
-        match self.groups.entry(gid) {
+        match self.groups.forward.entry(gid) {
             Vacant(entry) => {
                 let group = super::get_group_by_gid(gid);
                 match group {
                     Some(group) => {
                         entry.insert(Some(group.clone()));
-                        self.groups_back.insert(group.name.clone(), Some(group.gid));
+                        self.groups.backward.insert(group.name.clone(), Some(group.gid));
                         Some(group)
                     },
                     None => {
@@ -106,13 +120,13 @@ impl Users for OSUsers {
     fn get_group_by_name(&mut self, group_name: &str) -> Option<Group> {
         // to_owned() could change here:
         // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
-        match self.groups_back.entry(group_name.to_owned()) {
+        match self.groups.backward.entry(group_name.to_owned()) {
             Vacant(entry) => {
                 let user = super::get_group_by_name(group_name);
                 match user {
                     Some(group) => {
                         entry.insert(Some(group.gid));
-                        self.groups.insert(group.gid, Some(group.clone()));
+                        self.groups.forward.insert(group.gid, Some(group.clone()));
                         Some(group)
                     },
                     None => {
@@ -122,7 +136,7 @@ impl Users for OSUsers {
                 }
             },
             Occupied(entry) => match entry.get() {
-                &Some(gid) => self.groups[&gid].clone(),
+                &Some(gid) => self.groups.forward[&gid].clone(),
                 &None => None,
             }
         }

From aa94ee89dba075d9cb2b92afef632a3492562858 Mon Sep 17 00:00:00 2001
From: Benjamin Sago <ogham@bsago.me>
Date: Thu, 14 Jan 2016 23:06:14 -1000
Subject: [PATCH 5/7] Return Arcs instead of clones, and take &self

The primary goal of this commit was to avoid having to clone the users and groups it produces. The old implementation was written by me when I hadn't *fully* internalised how lifetimes worked, and just used `clone` to get around the error messages. It was only later that I realised how inefficient this actually is -- the values stored in the cache are never used, only their clones are!

Several attempt at this later, I settled on using `Arc` for everything and cloning *those* values instead.

All the gory details of what happened are written in a comment in the OS module. The short version is that it's now possible to change the `Users` trait's methods from taking `&mut self` to just taking `&self`. This is within the allowed uses of `Cell`: implementation details of a method that does actually return the same thing each time, which this is.

The main casualty is that the user now has to know about `Arc` and initialise a user with a username in such a container, which I hope to solve in a later commit.
---
 examples/example.rs   |   8 +-
 examples/threading.rs |  61 +++++++++++
 src/lib.rs            |  47 ++++-----
 src/mock.rs           |  81 ++++++++-------
 src/os.rs             | 228 +++++++++++++++++++++++++++++-------------
 5 files changed, 292 insertions(+), 133 deletions(-)
 create mode 100644 examples/threading.rs

diff --git a/examples/example.rs b/examples/example.rs
index 97741b8..204122a 100644
--- a/examples/example.rs
+++ b/examples/example.rs
@@ -2,14 +2,14 @@ extern crate users;
 use users::{Users, OSUsers};
 
 fn main() {
-	let mut cache = OSUsers::empty_cache();
-	
+	let cache = OSUsers::empty_cache();
+
 	let current_uid = cache.get_current_uid();
 	println!("Your UID is {}", current_uid);
-	
+
 	let you = cache.get_user_by_uid(current_uid).expect("No entry for current user!");
 	println!("Your username is {}", you.name);
-	
+
 	let primary_group = cache.get_group_by_gid(you.primary_group).expect("No entry for your primary group!");
 	println!("Your primary group has ID {} and name {}", primary_group.gid, primary_group.name);
 
diff --git a/examples/threading.rs b/examples/threading.rs
new file mode 100644
index 0000000..24ac1a3
--- /dev/null
+++ b/examples/threading.rs
@@ -0,0 +1,61 @@
+//! This example demonstrates how to use an `OSUsers` cache in a
+//! multi-threaded situation. The cache uses `RefCell`s internally, so it
+//! is distinctly not thread-safe. Instead, you’ll need to place it within
+//! some kind of lock in order to have threads access it one-at-a-time.
+//!
+//! It queries all the users it can find in the range 500..510. This is the
+//! default uid range on my Apple laptop -- Linux starts counting from 1000,
+//! but I can’t include both in the range! It spawns one thread per user to
+//! query, with each thread accessing the same cache.
+//!
+//! Then, afterwards, it retrieves references to the users that had been
+//! cached earlier.
+
+// For extra fun, try uncommenting some of the lines of code below, making
+// the code try to access the users cache *without* a Mutex, and see it
+// spew compile errors at you.
+
+extern crate users;
+use users::{Users, OSUsers, uid_t};
+
+use std::sync::{Arc, Mutex};
+use std::time::Duration;
+use std::thread;
+
+const LO: uid_t = 500;
+const HI: uid_t = 510;
+
+fn main() {
+
+    // For thread-safely, our users cache needs to be within a Mutex, so
+    // only one thread can access it once. This Mutex needs to be within an
+    // Arc, so multiple threads can access the Mutex.
+    let cache = Arc::new(Mutex::new(OSUsers::empty_cache()));
+    // let cache = OSUsers::empty_cache();
+
+    // Loop over the range and query all the users in the range. Although we
+    // could use the `&User` values returned, we just ignore them.
+    for uid in LO .. HI {
+        let cache = cache.clone();
+
+        thread::spawn(move || {
+            let cache = cache.lock().unwrap();  // Unlock the mutex
+            let _ = cache.get_user_by_uid(uid); // Query our users cache!
+        });
+    }
+
+    // Wait for all the threads to finish.
+    thread::sleep(Duration::from_millis(100));
+
+    // Loop over the same range and print out all the users we find.
+    // These users will be retrieved from the cache.
+    for uid in LO .. HI {
+        let cache = cache.lock().unwrap();             // Re-unlock the mutex
+        if let Some(u) = cache.get_user_by_uid(uid) {  // Re-query our cache!
+            println!("User #{} is {}", u.uid, u.name)
+        }
+        else {
+            println!("User #{} does not exist", uid);
+        }
+    }
+}
\ No newline at end of file
diff --git a/src/lib.rs b/src/lib.rs
index d38ff9d..e7a558b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -93,7 +93,7 @@
 //! let mut cache = OSUsers::empty_cache();
 //! let group = cache.get_group_by_name("admin").expect("No such group 'admin'!");
 //! println!("The '{}' group has the ID {}", group.name, group.gid);
-//! for member in group.members.into_iter() {
+//! for member in &group.members {
 //!     println!("{} is a member of the group", member);
 //! }
 //! ```
@@ -112,6 +112,7 @@ use std::ffi::{CStr, CString};
 use std::io::{Error as IOError, Result as IOResult};
 use std::ptr::read;
 use std::str::from_utf8_unchecked;
+use std::sync::Arc;
 
 extern crate libc;
 pub use libc::{uid_t, gid_t, c_int};
@@ -130,40 +131,40 @@ pub use os::OSUsers;
 pub trait Users {
 
     /// Return a User object if one exists for the given user ID; otherwise, return None.
-    fn get_user_by_uid(&mut self, uid: uid_t) -> Option<User>;
+    fn get_user_by_uid(&self, uid: uid_t) -> Option<Arc<User>>;
 
     /// Return a User object if one exists for the given username; otherwise, return None.
-    fn get_user_by_name(&mut self, username: &str) -> Option<User>;
+    fn get_user_by_name(&self, username: &str) -> Option<Arc<User>>;
 
     /// Return a Group object if one exists for the given group ID; otherwise, return None.
-    fn get_group_by_gid(&mut self, gid: gid_t) -> Option<Group>;
+    fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>>;
 
     /// Return a Group object if one exists for the given groupname; otherwise, return None.
-    fn get_group_by_name(&mut self, group_name: &str) -> Option<Group>;
+    fn get_group_by_name(&self, group_name: &str) -> Option<Arc<Group>>;
 
     /// Return the user ID for the user running the process.
-    fn get_current_uid(&mut self) -> uid_t;
+    fn get_current_uid(&self) -> uid_t;
 
     /// Return the username of the user running the process.
-    fn get_current_username(&mut self) -> Option<String>;
+    fn get_current_username(&self) -> Option<Arc<String>>;
 
     /// Return the group ID for the user running the process.
-    fn get_current_gid(&mut self) -> gid_t;
+    fn get_current_gid(&self) -> gid_t;
 
     /// Return the group name of the user running the process.
-    fn get_current_groupname(&mut self) -> Option<String>;
+    fn get_current_groupname(&self) -> Option<Arc<String>>;
 
     /// Return the effective user id.
-    fn get_effective_uid(&mut self) -> uid_t;
+    fn get_effective_uid(&self) -> uid_t;
 
     /// Return the effective group id.
-    fn get_effective_gid(&mut self) -> gid_t;
+    fn get_effective_gid(&self) -> gid_t;
 
     /// Return the effective username.
-    fn get_effective_username(&mut self) -> Option<String>;
+    fn get_effective_username(&self) -> Option<Arc<String>>;
 
     /// Return the effective group name.
-    fn get_effective_groupname(&mut self) -> Option<String>;
+    fn get_effective_groupname(&self) -> Option<Arc<String>>;
 }
 
 #[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "dragonfly"))]
@@ -232,7 +233,7 @@ pub struct User {
     pub uid: uid_t,
 
     /// This user's name
-    pub name: String,
+    pub name: Arc<String>,
 
     /// The ID of this user's primary group
     pub primary_group: gid_t,
@@ -252,7 +253,7 @@ pub struct Group {
     pub gid: uid_t,
 
     /// This group's name
-    pub name: String,
+    pub name: Arc<String>,
 
     /// Vector of the names of the users who belong to this group as a non-primary member
     pub members: Vec<String>,
@@ -267,7 +268,7 @@ unsafe fn passwd_to_user(pointer: *const c_passwd) -> Option<User> {
         let pw = read(pointer);
         Some(User {
             uid: pw.pw_uid as uid_t,
-            name: from_raw_buf(pw.pw_name as *const i8),
+            name: Arc::new(from_raw_buf(pw.pw_name as *const i8)),
             primary_group: pw.pw_gid as gid_t,
             home_dir: from_raw_buf(pw.pw_dir as *const i8),
             shell: from_raw_buf(pw.pw_shell as *const i8)
@@ -283,7 +284,7 @@ unsafe fn struct_to_group(pointer: *const c_group) -> Option<Group> {
         let gr = read(pointer);
         let name = from_raw_buf(gr.gr_name as *const i8);
         let members = members(gr.gr_mem);
-        Some(Group { gid: gr.gr_gid, name: name, members: members })
+        Some(Group { gid: gr.gr_gid, name: Arc::new(name), members: members })
     }
     else {
         None
@@ -355,7 +356,7 @@ pub fn get_current_uid() -> uid_t {
 /// Return the username of the user running the process.
 pub fn get_current_username() -> Option<String> {
     let uid = get_current_uid();
-    get_user_by_uid(uid).map(|u| u.name)
+    get_user_by_uid(uid).map(|u| Arc::try_unwrap(u.name).unwrap())
 }
 
 /// Return the user ID for the effective user running the process.
@@ -366,7 +367,7 @@ pub fn get_effective_uid() -> uid_t {
 /// Return the username of the effective user running the process.
 pub fn get_effective_username() -> Option<String> {
     let uid = get_effective_uid();
-    get_user_by_uid(uid).map(|u| u.name)
+    get_user_by_uid(uid).map(|u| Arc::try_unwrap(u.name).unwrap())
 }
 
 /// Return the group ID for the user running the process.
@@ -377,7 +378,7 @@ pub fn get_current_gid() -> gid_t {
 /// Return the groupname of the user running the process.
 pub fn get_current_groupname() -> Option<String> {
     let gid = get_current_gid();
-    get_group_by_gid(gid).map(|g| g.name)
+    get_group_by_gid(gid).map(|g| Arc::try_unwrap(g.name).unwrap())
 }
 
 /// Return the group ID for the effective user running the process.
@@ -388,7 +389,7 @@ pub fn get_effective_gid() -> gid_t {
 /// Return the groupname of the effective user running the process.
 pub fn get_effective_groupname() -> Option<String> {
     let gid = get_effective_gid();
-    get_group_by_gid(gid).map(|g| g.name)
+    get_group_by_gid(gid).map(|g| Arc::try_unwrap(g.name).unwrap())
 }
 
 /// Set current user for the running process, requires root priviledges.
@@ -497,7 +498,7 @@ mod test {
     #[test]
     fn username() {
         let uid = get_current_uid();
-        assert_eq!(get_current_username().unwrap(), get_user_by_uid(uid).unwrap().name);
+        assert_eq!(&*get_current_username().unwrap(), &*get_user_by_uid(uid).unwrap().name);
     }
 
     #[test]
@@ -531,7 +532,7 @@ mod test {
         let name = get_current_username().unwrap();
         let user_by_name = get_user_by_name(&name);
         assert!(user_by_name.is_some());
-        assert_eq!(user_by_name.unwrap().name, name);
+        assert_eq!(&**user_by_name.unwrap().name, &*name);
 
         // User names containing '\0' cannot be used (for now)
         let user = get_user_by_name("user\0");
diff --git a/src/mock.rs b/src/mock.rs
index 9fbb5e6..a5c613f 100644
--- a/src/mock.rs
+++ b/src/mock.rs
@@ -18,9 +18,11 @@
 //!
 //! ```rust
 //! use users::mock::{MockUsers, User, Group};
+//! use std::sync::Arc;
+//!
 //! let mut users = MockUsers::with_current_uid(1000);
-//! users.add_user(User { uid: 1000, name: "Bobbins".to_string(), primary_group: 100, home_dir: "/home/bobbins".to_string(), shell: "/bin/bash".to_string() });
-//! users.add_group(Group { gid: 100, name: "funkyppl".to_string(), members: vec![ "other_person".to_string() ] });
+//! users.add_user(User { uid: 1000, name: Arc::new("Bobbins".to_string()), primary_group: 100, home_dir: "/home/bobbins".to_string(), shell: "/bin/bash".to_string() });
+//! users.add_group(Group { gid: 100, name: Arc::new("funkyppl".to_string()), members: vec![ "other_person".to_string() ] });
 //! ```
 //!
 //! The exports get re-exported into the mock module, for simpler `use` lines.
@@ -37,13 +39,14 @@
 //! ```rust
 //! use users::{Users, OSUsers, User};
 //! use users::mock::MockUsers;
+//! use std::sync::Arc;
 //!
 //! fn print_current_username<U: Users>(users: &mut U) {
 //!     println!("Current user: {:?}", users.get_current_username());
 //! }
 //!
 //! let mut users = MockUsers::with_current_uid(1001);
-//! users.add_user(User { uid: 1001, name: "fred".to_string(), primary_group: 101 , home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string()});
+//! users.add_user(User { uid: 1001, name: Arc::new("fred".to_string()), primary_group: 101 , home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string()});
 //! print_current_username(&mut users);
 //!
 //! let mut actual_users = OSUsers::empty_cache();
@@ -52,12 +55,13 @@
 
 pub use super::{Users, User, Group};
 use std::collections::HashMap;
+use std::sync::Arc;
 use libc::{uid_t, gid_t};
 
 /// A mocking users object that you can add your own users and groups to.
 pub struct MockUsers {
-    users: HashMap<uid_t, User>,
-    groups: HashMap<gid_t, Group>,
+    users: HashMap<uid_t, Arc<User>>,
+    groups: HashMap<gid_t, Arc<Group>>,
     uid: uid_t,
 }
 
@@ -73,62 +77,62 @@ impl MockUsers {
     }
 
     /// Add a user to the users table.
-    pub fn add_user(&mut self, user: User) -> Option<User> {
-        self.users.insert(user.uid, user)
+    pub fn add_user(&mut self, user: User) -> Option<Arc<User>> {
+        self.users.insert(user.uid, Arc::new(user))
     }
 
     /// Add a group to the groups table.
-    pub fn add_group(&mut self, group: Group) -> Option<Group> {
-        self.groups.insert(group.gid, group)
+    pub fn add_group(&mut self, group: Group) -> Option<Arc<Group>> {
+        self.groups.insert(group.gid, Arc::new(group))
     }
 }
 
 impl Users for MockUsers {
-    fn get_user_by_uid(&mut self, uid: uid_t) -> Option<User> {
+    fn get_user_by_uid(&self, uid: uid_t) -> Option<Arc<User>> {
         self.users.get(&uid).cloned()
     }
 
-    fn get_user_by_name(&mut self, username: &str) -> Option<User> {
-        self.users.values().find(|u| u.name == username).cloned()
+    fn get_user_by_name(&self, username: &str) -> Option<Arc<User>> {
+        self.users.values().find(|u| &*u.name == username).cloned()
     }
 
-    fn get_group_by_gid(&mut self, gid: gid_t) -> Option<Group> {
+    fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>> {
         self.groups.get(&gid).cloned()
     }
 
-    fn get_group_by_name(&mut self, group_name: &str) -> Option<Group> {
-        self.groups.values().find(|g| g.name == group_name).cloned()
+    fn get_group_by_name(&self, group_name: &str) -> Option<Arc<Group>> {
+        self.groups.values().find(|g| &*g.name == group_name).cloned()
     }
 
-    fn get_current_uid(&mut self) -> uid_t {
+    fn get_current_uid(&self) -> uid_t {
         self.uid
     }
 
-    fn get_current_username(&mut self) -> Option<String> {
+    fn get_current_username(&self) -> Option<Arc<String>> {
         self.users.get(&self.uid).map(|u| u.name.clone())
     }
 
-    fn get_current_gid(&mut self) -> uid_t {
+    fn get_current_gid(&self) -> uid_t {
         self.uid
     }
 
-    fn get_current_groupname(&mut self) -> Option<String> {
+    fn get_current_groupname(&self) -> Option<Arc<String>> {
         self.groups.get(&self.uid).map(|u| u.name.clone())
     }
 
-    fn get_effective_uid(&mut self) -> uid_t {
+    fn get_effective_uid(&self) -> uid_t {
         self.uid
     }
 
-    fn get_effective_username(&mut self) -> Option<String> {
+    fn get_effective_username(&self) -> Option<Arc<String>> {
         self.users.get(&self.uid).map(|u| u.name.clone())
     }
 
-    fn get_effective_gid(&mut self) -> uid_t {
+    fn get_effective_gid(&self) -> uid_t {
         self.uid
     }
 
-    fn get_effective_groupname(&mut self) -> Option<String> {
+    fn get_effective_groupname(&self) -> Option<Arc<String>> {
         self.groups.get(&self.uid).map(|u| u.name.clone())
     }
 }
@@ -136,71 +140,72 @@ impl Users for MockUsers {
 #[cfg(test)]
 mod test {
     use super::{Users, User, Group, MockUsers};
+    use std::sync::Arc;
 
     #[test]
     fn current_username() {
         let mut users = MockUsers::with_current_uid(1337);
-        users.add_user(User { uid: 1337, name: "fred".to_string(), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
-        assert_eq!(Some("fred".to_string()), users.get_current_username())
+        users.add_user(User { uid: 1337, name: Arc::new("fred".to_string()), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
+        assert_eq!(Some(Arc::new("fred".into())), users.get_current_username())
     }
 
     #[test]
     fn no_current_username() {
-        let mut users = MockUsers::with_current_uid(1337);
+        let users = MockUsers::with_current_uid(1337);
         assert_eq!(None, users.get_current_username())
     }
 
     #[test]
     fn uid() {
         let mut users = MockUsers::with_current_uid(0);
-        users.add_user(User { uid: 1337, name: "fred".to_string(), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
-        assert_eq!(Some("fred".to_string()), users.get_user_by_uid(1337).map(|u| u.name))
+        users.add_user(User { uid: 1337, name: Arc::new("fred".to_string()), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
+        assert_eq!(Some(Arc::new("fred".into())), users.get_user_by_uid(1337).map(|u| u.name.clone()))
     }
 
     #[test]
     fn username() {
         let mut users = MockUsers::with_current_uid(1337);
-        users.add_user(User { uid: 1440, name: "fred".to_string(), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
+        users.add_user(User { uid: 1440, name: Arc::new("fred".to_string()), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
         assert_eq!(Some(1440), users.get_user_by_name("fred").map(|u| u.uid))
     }
 
     #[test]
     fn no_username() {
         let mut users = MockUsers::with_current_uid(1337);
-        users.add_user(User { uid: 1440, name: "fred".to_string(), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
+        users.add_user(User { uid: 1440, name: Arc::new("fred".to_string()), primary_group: 101, home_dir: "/home/fred".to_string(), shell: "/bin/bash".to_string() });
         assert_eq!(None, users.get_user_by_name("criminy").map(|u| u.uid))
     }
 
     #[test]
     fn no_uid() {
-        let mut users = MockUsers::with_current_uid(0);
-        assert_eq!(None, users.get_user_by_uid(1337).map(|u| u.name))
+        let users = MockUsers::with_current_uid(0);
+        assert_eq!(None, users.get_user_by_uid(1337).map(|u| u.name.clone()))
     }
 
     #[test]
     fn gid() {
         let mut users = MockUsers::with_current_uid(0);
-        users.add_group(Group { gid: 1337, name: "fred".to_string(), members: vec![], });
-        assert_eq!(Some("fred".to_string()), users.get_group_by_gid(1337).map(|g| g.name))
+        users.add_group(Group { gid: 1337, name: Arc::new("fred".to_string()), members: vec![], });
+        assert_eq!(Some(Arc::new("fred".into())), users.get_group_by_gid(1337).map(|g| g.name.clone()))
     }
 
     #[test]
     fn group_name() {
         let mut users = MockUsers::with_current_uid(0);
-        users.add_group(Group { gid: 1337, name: "fred".to_string(), members: vec![], });
+        users.add_group(Group { gid: 1337, name: Arc::new("fred".to_string()), members: vec![], });
         assert_eq!(Some(1337), users.get_group_by_name("fred").map(|g| g.gid))
     }
 
     #[test]
     fn no_group_name() {
         let mut users = MockUsers::with_current_uid(0);
-        users.add_group(Group { gid: 1337, name: "fred".to_string(), members: vec![], });
+        users.add_group(Group { gid: 1337, name: Arc::new("fred".to_string()), members: vec![], });
         assert_eq!(None, users.get_group_by_name("santa").map(|g| g.gid))
     }
 
     #[test]
     fn no_gid() {
-        let mut users = MockUsers::with_current_uid(0);
-        assert_eq!(None, users.get_group_by_gid(1337).map(|g| g.name))
+        let users = MockUsers::with_current_uid(0);
+        assert_eq!(None, users.get_group_by_gid(1337).map(|g| g.name.clone()))
     }
 }
diff --git a/src/os.rs b/src/os.rs
index 8cc6805..707fff9 100644
--- a/src/os.rs
+++ b/src/os.rs
@@ -1,21 +1,85 @@
+//! A cache for users and groups provided by the OS.
+//!
+//! ## Caching, multiple threads, and mutability
+//!
+//! The `OSUsers` type is caught between a rock and a hard place when it comes
+//! to providing references to users and groups.
+//!
+//! Instead of returning a fresh `User` struct each time, for example, it will
+//! return a reference to the version it currently has in its cache. So you can
+//! ask for User #501 twice, and you’ll get a reference to the same value both
+//! time. Its methods are *idempotent* -- calling one multiple times has the
+//! same effect as calling one once.
+//!
+//! This works fine in theory, but in practice, the cache has to update its own
+//! state somehow: it contains several `HashMap`s that hold the result of user
+//! and group lookups. Rust provides mutability in two ways:
+//!
+//! 1. Have its methods take `&mut self`, instead of `&self`, allowing the
+//!   internal maps to be mutated (“inherited mutability”)
+//! 2. Wrap the internal maps in a `RefCell`, allowing them to be modified
+//!   (“interior mutability”).
+//!
+//! Unfortunately, Rust is also very protective of references to a mutable
+//! value. In this case, switching to `&mut self` would only allow for one user
+//! to be read at a time!
+//!
+//! ``norun
+//! let mut cache = OSUsers::empty_cache();
+//! let uid   = cache.get_current_uid();                     // OK...
+//! let user  = cache.get_user_by_uid(uid).unwrap()          // OK...
+//! let group = cache.get_group_by_gid(user.primary_group);  // No!
+//! ```
+//!
+//! When we get the `user`, it returns an optional reference (which we unwrap)
+//! to the user’s entry in the cache. This is a reference to something contained
+//! in a mutable value. Then, when we want to get the user’s primary group, it
+//! will return *another* reference to the same mutable value. This is something
+//! that Rust explicitly disallows!
+//!
+//! The compiler wasn’t on our side with Option 1, so let’s try Option 2:
+//! changing the methods back to `&self` instead of `&mut self`, and using
+//! `RefCell`s internally. However, Rust is smarter than this, and knows that
+//! we’re just trying the same trick as earlier. A simplified implementation of
+//! a user cache lookup would look something like this:
+//!
+//! ``norun
+//! fn get_user_by_uid(&self, uid: uid_t) -> Option<&User> {
+//!     let users = self.users.borrow_mut();
+//!     users.get(uid)
+//! }
+//! ```
+//!
+//! Rust won’t allow us to return a reference like this because the `Ref` of the
+//! `RefCell` just gets dropped at the end of the method, meaning that our
+//! reference does not live long enough.
+//!
+//! So instead of doing any of that, we use `Arc` everywhere in order to get
+//! around all the lifetime restrictions. Returning reference-counted users and
+//! groups mean that we don’t have to worry about further uses of the cache, as
+//! the values themselves don’t count as being stored *in* the cache anymore. So
+//! it can be queried multiple times or go out of scope and the values it
+//! produces are not affected.
+
 use libc::{uid_t, gid_t};
 use std::borrow::ToOwned;
+use std::cell::{Cell, RefCell};
 use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::collections::HashMap;
+use std::sync::Arc;
 
 use super::{User, Group, Users};
 
 
 /// A producer of user and group instances that caches every result.
-#[derive(Clone)]
 pub struct OSUsers {
-    users: BiMap<uid_t, User>,
+    users:  BiMap<uid_t, User>,
     groups: BiMap<gid_t, Group>,
 
-    uid: Option<uid_t>,
-    gid: Option<gid_t>,
-    euid: Option<uid_t>,
-    egid: Option<gid_t>,
+    uid:  Cell<Option<uid_t>>,
+    gid:  Cell<Option<gid_t>>,
+    euid: Cell<Option<uid_t>>,
+    egid: Cell<Option<gid_t>>,
 }
 
 /// A kinda-bi-directional HashMap that associates keys to values, and then
@@ -23,10 +87,9 @@ pub struct OSUsers {
 /// *values*-to-keys lookup, because we only want to search based on
 /// usernames and group names. There wouldn’t be much point offering a “User
 /// to uid” map, as the uid is present in the user struct!
-#[derive(Clone)]
 struct BiMap<K, V> {
-    forward:  HashMap<K, Option<V>>,
-    backward: HashMap<String, Option<K>>,
+    forward:  RefCell< HashMap<K, Option<Arc<V>>> >,
+    backward: RefCell< HashMap<Arc<String>, Option<K>> >,
 }
 
 impl OSUsers {
@@ -34,33 +97,38 @@ impl OSUsers {
     pub fn empty_cache() -> OSUsers {
         OSUsers {
             users: BiMap {
-                forward:  HashMap::new(),
-                backward: HashMap::new(),
+                forward:  RefCell::new(HashMap::new()),
+                backward: RefCell::new(HashMap::new()),
             },
 
             groups: BiMap {
-                forward:  HashMap::new(),
-                backward: HashMap::new(),
+                forward:  RefCell::new(HashMap::new()),
+                backward: RefCell::new(HashMap::new()),
             },
 
-            uid:         None,
-            gid:         None,
-            euid:        None,
-            egid:        None,
+            uid:  Cell::new(None),
+            gid:  Cell::new(None),
+            euid: Cell::new(None),
+            egid: Cell::new(None),
         }
     }
 }
 
 impl Users for OSUsers {
-    fn get_user_by_uid(&mut self, uid: uid_t) -> Option<User> {
-        match self.users.forward.entry(uid) {
+    fn get_user_by_uid(&self, uid: uid_t) -> Option<Arc<User>> {
+        let mut users_forward = self.users.forward.borrow_mut();
+
+        match users_forward.entry(uid) {
             Vacant(entry) => {
-                let user = super::get_user_by_uid(uid);
-                match user {
+                match super::get_user_by_uid(uid) {
                     Some(user) => {
-                        entry.insert(Some(user.clone()));
-                        self.users.backward.insert(user.name.clone(), Some(user.uid));
-                        Some(user)
+                        let newsername = user.name.clone();
+                        let mut users_backward = self.users.backward.borrow_mut();
+                        users_backward.insert(newsername, Some(uid));
+
+                        let user_arc = Arc::new(user);
+                        entry.insert(Some(user_arc.clone()));
+                        Some(user_arc)
                     },
                     None => {
                         entry.insert(None);
@@ -72,17 +140,23 @@ impl Users for OSUsers {
         }
     }
 
-    fn get_user_by_name(&mut self, username: &str) -> Option<User> {
+    fn get_user_by_name(&self, username: &str) -> Option<Arc<User>> {
+        let mut users_backward = self.users.backward.borrow_mut();
+
         // to_owned() could change here:
         // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
-        match self.users.backward.entry(username.to_owned()) {
+        match users_backward.entry(Arc::new(username.to_owned())) {
             Vacant(entry) => {
-                let user = super::get_user_by_name(username);
-                match user {
+                match super::get_user_by_name(username) {
                     Some(user) => {
-                        entry.insert(Some(user.uid));
-                        self.users.forward.insert(user.uid, Some(user.clone()));
-                        Some(user)
+                        let uid = user.uid;
+                        let user_arc = Arc::new(user);
+
+                        let mut users_forward = self.users.forward.borrow_mut();
+                        users_forward.insert(uid, Some(user_arc.clone()));
+                        entry.insert(Some(uid));
+
+                        Some(user_arc)
                     },
                     None => {
                         entry.insert(None);
@@ -90,22 +164,31 @@ impl Users for OSUsers {
                     }
                 }
             },
-            Occupied(entry) => match entry.get() {
-                &Some(uid) => self.users.forward[&uid].clone(),
-                &None => None,
+            Occupied(entry) => match *entry.get() {
+                Some(uid) => {
+                    let users_forward = self.users.forward.borrow_mut();
+                    users_forward[&uid].clone()
+                }
+                None => None,
             }
         }
     }
 
-    fn get_group_by_gid(&mut self, gid: gid_t) -> Option<Group> {
-        match self.groups.forward.entry(gid) {
+    fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>> {
+        let mut groups_forward = self.groups.forward.borrow_mut();
+
+        match groups_forward.entry(gid) {
             Vacant(entry) => {
                 let group = super::get_group_by_gid(gid);
                 match group {
                     Some(group) => {
-                        entry.insert(Some(group.clone()));
-                        self.groups.backward.insert(group.name.clone(), Some(group.gid));
-                        Some(group)
+                        let new_group_name = group.name.clone();
+                        let mut groups_backward = self.groups.backward.borrow_mut();
+                        groups_backward.insert(new_group_name, Some(gid));
+
+                        let group_arc = Arc::new(group);
+                        entry.insert(Some(group_arc.clone()));
+                        Some(group_arc)
                     },
                     None => {
                         entry.insert(None);
@@ -117,17 +200,24 @@ impl Users for OSUsers {
         }
     }
 
-    fn get_group_by_name(&mut self, group_name: &str) -> Option<Group> {
+    fn get_group_by_name(&self, group_name: &str) -> Option<Arc<Group>> {
+        let mut groups_backward = self.groups.backward.borrow_mut();
+
         // to_owned() could change here:
         // https://github.com/rust-lang/rfcs/blob/master/text/0509-collections-reform-part-2.md#alternatives-to-toowned-on-entries
-        match self.groups.backward.entry(group_name.to_owned()) {
+        match groups_backward.entry(Arc::new(group_name.to_owned())) {
             Vacant(entry) => {
                 let user = super::get_group_by_name(group_name);
                 match user {
                     Some(group) => {
-                        entry.insert(Some(group.gid));
-                        self.groups.forward.insert(group.gid, Some(group.clone()));
-                        Some(group)
+                        let group_arc = Arc::new(group.clone());
+                        let gid = group.gid;
+
+                        let mut groups_forward = self.groups.forward.borrow_mut();
+                        groups_forward.insert(gid, Some(group_arc.clone()));
+                        entry.insert(Some(gid));
+
+                        Some(group_arc)
                     },
                     None => {
                         entry.insert(None);
@@ -135,75 +225,77 @@ impl Users for OSUsers {
                     }
                 }
             },
-            Occupied(entry) => match entry.get() {
-                &Some(gid) => self.groups.forward[&gid].clone(),
-                &None => None,
+            Occupied(entry) => match *entry.get() {
+                Some(gid) => {
+                    let groups_forward = self.groups.forward.borrow_mut();
+                    groups_forward[&gid].as_ref().cloned()
+                }
+                None => None,
             }
         }
     }
 
-    fn get_current_uid(&mut self) -> uid_t {
-        match self.uid {
+    fn get_current_uid(&self) -> uid_t {
+        match self.uid.get() {
             Some(uid) => uid,
             None => {
                 let uid = super::get_current_uid();
-                self.uid = Some(uid);
+                self.uid.set(Some(uid));
                 uid
             }
         }
     }
 
-    /// Return the username of the user running the process.
-    fn get_current_username(&mut self) -> Option<String> {
+    fn get_current_username(&self) -> Option<Arc<String>> {
         let uid = self.get_current_uid();
-        self.get_user_by_uid(uid).map(|u| u.name)
+        self.get_user_by_uid(uid).map(|u| u.name.clone())
     }
 
-    fn get_current_gid(&mut self) -> gid_t {
-        match self.gid {
+    fn get_current_gid(&self) -> gid_t {
+        match self.gid.get() {
             Some(gid) => gid,
             None => {
                 let gid = super::get_current_gid();
-                self.gid = Some(gid);
+                self.gid.set(Some(gid));
                 gid
             }
         }
     }
 
-    fn get_current_groupname(&mut self) -> Option<String> {
+    fn get_current_groupname(&self) -> Option<Arc<String>> {
         let gid = self.get_current_gid();
-        self.get_group_by_gid(gid).map(|g| g.name)
+        self.get_group_by_gid(gid).map(|g| g.name.clone())
     }
 
-    fn get_effective_gid(&mut self) -> gid_t {
-        match self.egid {
+    fn get_effective_gid(&self) -> gid_t {
+        match self.egid.get() {
             Some(gid) => gid,
             None => {
                 let gid = super::get_effective_gid();
-                self.egid = Some(gid);
+                self.egid.set(Some(gid));
                 gid
             }
         }
     }
 
-    fn get_effective_groupname(&mut self) -> Option<String> {
+    fn get_effective_groupname(&self) -> Option<Arc<String>> {
         let gid = self.get_effective_gid();
-        self.get_group_by_gid(gid).map(|g| g.name)
+        self.get_group_by_gid(gid).map(|g| g.name.clone())
     }
 
-    fn get_effective_uid(&mut self) -> uid_t {
-        match self.euid {
+    fn get_effective_uid(&self) -> uid_t {
+        match self.euid.get() {
             Some(uid) => uid,
             None => {
                 let uid = super::get_effective_uid();
-                self.euid = Some(uid);
+                self.euid.set(Some(uid));
                 uid
             }
         }
     }
 
-    fn get_effective_username(&mut self) -> Option<String> {
+    fn get_effective_username(&self) -> Option<Arc<String>> {
         let uid = self.get_effective_uid();
-        self.get_user_by_uid(uid).map(|u| u.name)
+        self.get_user_by_uid(uid).map(|u| u.name.clone())
     }
 }

From 052cf245f40c6cbf15c99c799fff82312b3082e6 Mon Sep 17 00:00:00 2001
From: Benjamin Sago <ogham@bsago.me>
Date: Thu, 14 Jan 2016 23:15:53 -1000
Subject: [PATCH 6/7] Split Users trait into Users and Groups traits

It was getting crowded in there! Not only are two groups of six easier to manage than one group of twelve, but the groups functions didn't actually have anything to do with users...
---
 examples/example.rs |  2 +-
 src/lib.rs          | 82 ++++++++++++++++++++++++---------------------
 src/mock.rs         | 38 +++++++++++----------
 src/os.rs           | 68 +++++++++++++++++++------------------
 4 files changed, 99 insertions(+), 91 deletions(-)

diff --git a/examples/example.rs b/examples/example.rs
index 204122a..069d0ef 100644
--- a/examples/example.rs
+++ b/examples/example.rs
@@ -1,5 +1,5 @@
 extern crate users;
-use users::{Users, OSUsers};
+use users::{Users, Groups, OSUsers};
 
 fn main() {
 	let cache = OSUsers::empty_cache();
diff --git a/src/lib.rs b/src/lib.rs
index e7a558b..92a6875 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -64,7 +64,7 @@
 //! methods on it. For example:
 //!
 //! ```rust
-//! use users::{Users, OSUsers};
+//! use users::{Users, Groups, OSUsers};
 //! let mut cache = OSUsers::empty_cache();
 //! let uid = cache.get_current_uid();
 //! let user = cache.get_user_by_uid(uid).unwrap();
@@ -89,7 +89,7 @@
 //! And again, a complete example:
 //!
 //! ```rust
-//! use users::{Users, OSUsers};
+//! use users::{Users, Groups, OSUsers};
 //! let mut cache = OSUsers::empty_cache();
 //! let group = cache.get_group_by_name("admin").expect("No such group 'admin'!");
 //! println!("The '{}' group has the ID {}", group.name, group.gid);
@@ -127,43 +127,47 @@ pub mod mock;
 pub mod os;
 pub use os::OSUsers;
 
-/// The trait for the `OSUsers` object.
+/// Trait for producers of users.
 pub trait Users {
 
-    /// Return a User object if one exists for the given user ID; otherwise, return None.
+    /// Returns a User if one exists for the given user ID; otherwise, returns None.
     fn get_user_by_uid(&self, uid: uid_t) -> Option<Arc<User>>;
 
-    /// Return a User object if one exists for the given username; otherwise, return None.
+    /// Returns a User if one exists for the given username; otherwise, returns None.
     fn get_user_by_name(&self, username: &str) -> Option<Arc<User>>;
 
-    /// Return a Group object if one exists for the given group ID; otherwise, return None.
-    fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>>;
-
-    /// Return a Group object if one exists for the given groupname; otherwise, return None.
-    fn get_group_by_name(&self, group_name: &str) -> Option<Arc<Group>>;
-
-    /// Return the user ID for the user running the process.
+    /// Returns the user ID for the user running the process.
     fn get_current_uid(&self) -> uid_t;
 
-    /// Return the username of the user running the process.
+    /// Returns the username of the user running the process.
     fn get_current_username(&self) -> Option<Arc<String>>;
 
-    /// Return the group ID for the user running the process.
-    fn get_current_gid(&self) -> gid_t;
-
-    /// Return the group name of the user running the process.
-    fn get_current_groupname(&self) -> Option<Arc<String>>;
-
-    /// Return the effective user id.
+    /// Returns the effective user id.
     fn get_effective_uid(&self) -> uid_t;
 
-    /// Return the effective group id.
+    /// Returns the effective username.
+    fn get_effective_username(&self) -> Option<Arc<String>>;
+}
+
+/// Trait for producers of groups.
+pub trait Groups {
+
+    /// Returns a Group object if one exists for the given group ID; otherwise, returns None.
+    fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>>;
+
+    /// Returns a Group object if one exists for the given groupname; otherwise, returns None.
+    fn get_group_by_name(&self, group_name: &str) -> Option<Arc<Group>>;
+
+    /// Returns the group ID for the user running the process.
+    fn get_current_gid(&self) -> gid_t;
+
+    /// Returns the group name of the user running the process.
+    fn get_current_groupname(&self) -> Option<Arc<String>>;
+
+    /// Returns the effective group id.
     fn get_effective_gid(&self) -> gid_t;
 
-    /// Return the effective username.
-    fn get_effective_username(&self) -> Option<Arc<String>>;
-
-    /// Return the effective group name.
+    /// Returns the effective group name.
     fn get_effective_groupname(&self) -> Option<Arc<String>>;
 }
 
@@ -225,8 +229,8 @@ extern {
     fn setregid(rgid: gid_t, egid: gid_t) -> c_int;
 }
 
-#[derive(Clone)]
 /// Information about a particular user.
+#[derive(Clone)]
 pub struct User {
 
     /// This user's ID
@@ -312,12 +316,12 @@ unsafe fn members(groups: *const *const c_char) -> Vec<String> {
 }
 
 
-/// Return a User object if one exists for the given user ID; otherwise, return None.
+/// Returns a User object if one exists for the given user ID; otherwise, return None.
 pub fn get_user_by_uid(uid: uid_t) -> Option<User> {
     unsafe { passwd_to_user(getpwuid(uid)) }
 }
 
-/// Return a User object if one exists for the given username; otherwise, return None.
+/// Returns a User object if one exists for the given username; otherwise, return None.
 pub fn get_user_by_name(username: &str) -> Option<User> {
     let username_c = CString::new(username);
 
@@ -330,12 +334,12 @@ pub fn get_user_by_name(username: &str) -> Option<User> {
     unsafe { passwd_to_user(getpwnam(username_c.unwrap().as_ptr())) }
 }
 
-/// Return a Group object if one exists for the given group ID; otherwise, return None.
+/// Returns a Group object if one exists for the given group ID; otherwise, return None.
 pub fn get_group_by_gid(gid: gid_t) -> Option<Group> {
     unsafe { struct_to_group(getgrgid(gid)) }
 }
 
-/// Return a Group object if one exists for the given groupname; otherwise, return None.
+/// Returns a Group object if one exists for the given groupname; otherwise, return None.
 pub fn get_group_by_name(group_name: &str) -> Option<Group> {
     let group_name_c = CString::new(group_name);
 
@@ -348,51 +352,51 @@ pub fn get_group_by_name(group_name: &str) -> Option<Group> {
     unsafe { struct_to_group(getgrnam(group_name_c.unwrap().as_ptr())) }
 }
 
-/// Return the user ID for the user running the process.
+/// Returns the user ID for the user running the process.
 pub fn get_current_uid() -> uid_t {
     unsafe { getuid() }
 }
 
-/// Return the username of the user running the process.
+/// Returns the username of the user running the process.
 pub fn get_current_username() -> Option<String> {
     let uid = get_current_uid();
     get_user_by_uid(uid).map(|u| Arc::try_unwrap(u.name).unwrap())
 }
 
-/// Return the user ID for the effective user running the process.
+/// Returns the user ID for the effective user running the process.
 pub fn get_effective_uid() -> uid_t {
     unsafe { geteuid() }
 }
 
-/// Return the username of the effective user running the process.
+/// Returns the username of the effective user running the process.
 pub fn get_effective_username() -> Option<String> {
     let uid = get_effective_uid();
     get_user_by_uid(uid).map(|u| Arc::try_unwrap(u.name).unwrap())
 }
 
-/// Return the group ID for the user running the process.
+/// Returns the group ID for the user running the process.
 pub fn get_current_gid() -> gid_t {
     unsafe { getgid() }
 }
 
-/// Return the groupname of the user running the process.
+/// Returns the groupname of the user running the process.
 pub fn get_current_groupname() -> Option<String> {
     let gid = get_current_gid();
     get_group_by_gid(gid).map(|g| Arc::try_unwrap(g.name).unwrap())
 }
 
-/// Return the group ID for the effective user running the process.
+/// Returns the group ID for the effective user running the process.
 pub fn get_effective_gid() -> gid_t {
     unsafe { getegid() }
 }
 
-/// Return the groupname of the effective user running the process.
+/// Returns the groupname of the effective user running the process.
 pub fn get_effective_groupname() -> Option<String> {
     let gid = get_effective_gid();
     get_group_by_gid(gid).map(|g| Arc::try_unwrap(g.name).unwrap())
 }
 
-/// Set current user for the running process, requires root priviledges.
+/// Sets current user for the running process, requires root priviledges.
 pub fn set_current_uid(uid: uid_t) -> IOResult<()> {
     match unsafe { setuid(uid) } {
         0 => Ok(()),
diff --git a/src/mock.rs b/src/mock.rs
index a5c613f..5480b02 100644
--- a/src/mock.rs
+++ b/src/mock.rs
@@ -53,7 +53,7 @@
 //! print_current_username(&mut actual_users);
 //! ```
 
-pub use super::{Users, User, Group};
+pub use super::{Users, Groups, User, Group};
 use std::collections::HashMap;
 use std::sync::Arc;
 use libc::{uid_t, gid_t};
@@ -96,14 +96,6 @@ impl Users for MockUsers {
         self.users.values().find(|u| &*u.name == username).cloned()
     }
 
-    fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>> {
-        self.groups.get(&gid).cloned()
-    }
-
-    fn get_group_by_name(&self, group_name: &str) -> Option<Arc<Group>> {
-        self.groups.values().find(|g| &*g.name == group_name).cloned()
-    }
-
     fn get_current_uid(&self) -> uid_t {
         self.uid
     }
@@ -112,14 +104,6 @@ impl Users for MockUsers {
         self.users.get(&self.uid).map(|u| u.name.clone())
     }
 
-    fn get_current_gid(&self) -> uid_t {
-        self.uid
-    }
-
-    fn get_current_groupname(&self) -> Option<Arc<String>> {
-        self.groups.get(&self.uid).map(|u| u.name.clone())
-    }
-
     fn get_effective_uid(&self) -> uid_t {
         self.uid
     }
@@ -127,6 +111,24 @@ impl Users for MockUsers {
     fn get_effective_username(&self) -> Option<Arc<String>> {
         self.users.get(&self.uid).map(|u| u.name.clone())
     }
+}
+
+impl Groups for MockUsers {
+    fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>> {
+        self.groups.get(&gid).cloned()
+    }
+
+    fn get_group_by_name(&self, group_name: &str) -> Option<Arc<Group>> {
+        self.groups.values().find(|g| &*g.name == group_name).cloned()
+    }
+
+    fn get_current_gid(&self) -> uid_t {
+        self.uid
+    }
+
+    fn get_current_groupname(&self) -> Option<Arc<String>> {
+        self.groups.get(&self.uid).map(|u| u.name.clone())
+    }
 
     fn get_effective_gid(&self) -> uid_t {
         self.uid
@@ -139,7 +141,7 @@ impl Users for MockUsers {
 
 #[cfg(test)]
 mod test {
-    use super::{Users, User, Group, MockUsers};
+    use super::{Users, Groups, User, Group, MockUsers};
     use std::sync::Arc;
 
     #[test]
diff --git a/src/os.rs b/src/os.rs
index 707fff9..5185ef0 100644
--- a/src/os.rs
+++ b/src/os.rs
@@ -68,7 +68,7 @@ use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::collections::HashMap;
 use std::sync::Arc;
 
-use super::{User, Group, Users};
+use super::{User, Groups, Group, Users};
 
 
 /// A producer of user and group instances that caches every result.
@@ -174,6 +174,40 @@ impl Users for OSUsers {
         }
     }
 
+    fn get_current_uid(&self) -> uid_t {
+        match self.uid.get() {
+            Some(uid) => uid,
+            None => {
+                let uid = super::get_current_uid();
+                self.uid.set(Some(uid));
+                uid
+            }
+        }
+    }
+
+    fn get_current_username(&self) -> Option<Arc<String>> {
+        let uid = self.get_current_uid();
+        self.get_user_by_uid(uid).map(|u| u.name.clone())
+    }
+
+    fn get_effective_uid(&self) -> uid_t {
+        match self.euid.get() {
+            Some(uid) => uid,
+            None => {
+                let uid = super::get_effective_uid();
+                self.euid.set(Some(uid));
+                uid
+            }
+        }
+    }
+
+    fn get_effective_username(&self) -> Option<Arc<String>> {
+        let uid = self.get_effective_uid();
+        self.get_user_by_uid(uid).map(|u| u.name.clone())
+    }
+}
+
+impl Groups for OSUsers {
     fn get_group_by_gid(&self, gid: gid_t) -> Option<Arc<Group>> {
         let mut groups_forward = self.groups.forward.borrow_mut();
 
@@ -235,22 +269,6 @@ impl Users for OSUsers {
         }
     }
 
-    fn get_current_uid(&self) -> uid_t {
-        match self.uid.get() {
-            Some(uid) => uid,
-            None => {
-                let uid = super::get_current_uid();
-                self.uid.set(Some(uid));
-                uid
-            }
-        }
-    }
-
-    fn get_current_username(&self) -> Option<Arc<String>> {
-        let uid = self.get_current_uid();
-        self.get_user_by_uid(uid).map(|u| u.name.clone())
-    }
-
     fn get_current_gid(&self) -> gid_t {
         match self.gid.get() {
             Some(gid) => gid,
@@ -282,20 +300,4 @@ impl Users for OSUsers {
         let gid = self.get_effective_gid();
         self.get_group_by_gid(gid).map(|g| g.name.clone())
     }
-
-    fn get_effective_uid(&self) -> uid_t {
-        match self.euid.get() {
-            Some(uid) => uid,
-            None => {
-                let uid = super::get_effective_uid();
-                self.euid.set(Some(uid));
-                uid
-            }
-        }
-    }
-
-    fn get_effective_username(&self) -> Option<Arc<String>> {
-        let uid = self.get_effective_uid();
-        self.get_user_by_uid(uid).map(|u| u.name.clone())
-    }
 }

From 28d4d68daf541734896f2bc260b14907838e77bd Mon Sep 17 00:00:00 2001
From: Benjamin Sago <ogham@bsago.me>
Date: Sat, 16 Jan 2016 11:27:18 -1000
Subject: [PATCH 7/7] Add Default impl for OSUsers

---
 src/os.rs | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/os.rs b/src/os.rs
index 5185ef0..dd92427 100644
--- a/src/os.rs
+++ b/src/os.rs
@@ -92,9 +92,12 @@ struct BiMap<K, V> {
     backward: RefCell< HashMap<Arc<String>, Option<K>> >,
 }
 
-impl OSUsers {
-    /// Create a new empty OS Users object.
-    pub fn empty_cache() -> OSUsers {
+// Default has to be impl'd manually here, because there's no
+// Default impl on User or Group, even though those types aren't
+// needed to produce a default instance of any HashMaps...
+
+impl Default for OSUsers {
+    fn default() -> OSUsers {
         OSUsers {
             users: BiMap {
                 forward:  RefCell::new(HashMap::new()),
@@ -114,6 +117,14 @@ impl OSUsers {
     }
 }
 
+impl OSUsers {
+
+    /// Create a new empty cache.
+    pub fn empty_cache() -> OSUsers {
+        OSUsers::default()
+    }
+}
+
 impl Users for OSUsers {
     fn get_user_by_uid(&self, uid: uid_t) -> Option<Arc<User>> {
         let mut users_forward = self.users.forward.borrow_mut();