From 0846f15947d20353ced0341356f08389c11341c9 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Mon, 19 Aug 2019 14:33:00 -0300 Subject: [PATCH 01/17] Add `Users` and `Group` related functions This was a collaborative work between Johannes Schilling , Fredrick Brennan and myself. Signed-off-by: Otavio Salvador --- src/unistd.rs | 431 +++++++++++++++++++++++++++++++++++++++++++- test/test.rs | 2 + test/test_unistd.rs | 56 ++++++ 3 files changed, 487 insertions(+), 2 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index f422f09198..e8e4d2d05c 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -20,12 +20,17 @@ pub use self::pivot_root::*; #[cfg(any(target_os = "android", target_os = "freebsd", target_os = "linux", target_os = "openbsd"))] pub use self::setres::*; +#[cfg(not(any(target_os = "android", + target_os = "ios", + target_os = "macos", + target_env = "musl")))] +pub use self::usergroupiter::*; /// User identifier /// /// Newtype pattern around `uid_t` (which is just alias). It prevents bugs caused by accidentally /// passing wrong value. -#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Default)] pub struct Uid(uid_t); impl Uid { @@ -74,7 +79,7 @@ pub const ROOT: Uid = Uid(0); /// /// Newtype pattern around `gid_t` (which is just alias). It prevents bugs caused by accidentally /// passing wrong value. -#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Default)] pub struct Gid(gid_t); impl Gid { @@ -2392,3 +2397,425 @@ pub fn access(path: &P, amode: AccessFlags) -> Result<()> { })?; Errno::result(res).map(drop) } + +/// Representation of a User, based on `libc::passwd` +/// +/// The reason some fields in this struct are `String` and others are `CString` is because some +/// fields are based on the user's locale, which could be non-UTF8, while other fields are +/// guaranteed to conform to [NAME_REGEX](https://serverfault.com/a/73101/407341), which only +/// contains ASCII. +#[derive(Debug, Clone, PartialEq, Default)] +pub struct User { + /// Username + pub name: String, + /// User password (probably encrypted) + pub passwd: CString, + /// User ID + pub uid: Uid, + /// Group ID + pub gid: Gid, + /// User information + #[cfg(not(target_os = "android"))] + pub gecos: CString, + /// Home directory + pub dir: PathBuf, + /// Path to shell + pub shell: PathBuf, + /// Login class + #[cfg(not(any(target_os = "android", target_os = "linux")))] + pub class: CString, + /// Last password change + #[cfg(not(any(target_os = "android", target_os = "linux")))] + pub change: libc::time_t, + /// Expiration time of account + #[cfg(not(any(target_os = "android", target_os = "linux")))] + pub expire: libc::time_t +} + +impl From<*mut libc::passwd> for User { + fn from(pw: *mut libc::passwd) -> User { + unsafe { + User { + name: CStr::from_ptr((*pw).pw_name).to_string_lossy().into_owned(), + passwd: CString::new(CStr::from_ptr((*pw).pw_passwd).to_bytes()).unwrap(), + #[cfg(not(target_os = "android"))] + gecos: CString::new(CStr::from_ptr((*pw).pw_gecos).to_bytes()).unwrap(), + dir: PathBuf::from(OsStr::from_bytes(CStr::from_ptr((*pw).pw_dir).to_bytes())), + shell: PathBuf::from(OsStr::from_bytes(CStr::from_ptr((*pw).pw_shell).to_bytes())), + uid: Uid::from_raw((*pw).pw_uid), + gid: Gid::from_raw((*pw).pw_gid), + #[cfg(not(any(target_os = "android", target_os = "linux")))] + class: CString::new(CStr::from_ptr((*pw).pw_class).to_bytes()).unwrap(), + #[cfg(not(any(target_os = "android", target_os = "linux")))] + change: (*pw).pw_change, + #[cfg(not(any(target_os = "android", target_os = "linux")))] + expire: (*pw).pw_expire + } + } + } +} + +/// Representation of a Group, based on `libc::group` +#[derive(Debug, Clone, PartialEq)] +pub struct Group { + /// Group name + pub name: String, + /// Group ID + pub gid: Gid, + /// List of Group members + pub mem: Vec +} + +impl From<*mut libc::group> for Group { + fn from(gr: *mut libc::group) -> Group { + unsafe { + Group { + name: CStr::from_ptr((*gr).gr_name).to_string_lossy().into_owned(), + gid: Gid::from_raw((*gr).gr_gid), + mem: Group::members((*gr).gr_mem) + } + } + } +} + +impl Group { + unsafe fn members(mem: *mut *mut c_char) -> Vec { + let mut ret = vec![]; + + for i in 0.. { + let u = mem.offset(i); + if (*u).is_null() { + break; + } else { + let s = CStr::from_ptr(*u).to_string_lossy().into_owned(); + ret.push(s); + } + } + + ret + } +} + +/// Query a user. +/// +/// The buffer size variants should not be needed 99% of the time; the default buffer size of 1024 +/// should be more than adequate. Only use the buffer size variants if you receive an ERANGE error +/// without them, or you know that you will be likely to. +/// +/// # Examples +/// +/// ``` +/// use nix::unistd::{User, UserQuery, Queryable}; +/// let find = String::from("root"); +/// // Returns an Option>, thus the double unwrap. +/// let res = User::query( UserQuery::Name(find.clone()) ).unwrap().unwrap(); +/// assert!(res.name == find); +/// ``` +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub enum UserQuery { + /// Get a user by UID. + /// + /// Internally, this function calls + /// [getpwuid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + Uid(Uid), + /// Get a user by name. + /// + /// Internally, this function calls + /// [getpwnam_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + Name(String), + UidWithBufsize(Uid, usize), + NameWithBufsize(String, usize) +} + +/// Query a group. +/// +/// The buffer size variants should not be needed 99% of the time; the default buffer size of 1024 +/// should be more than adequate. Only use the buffer size variants if you receive an ERANGE error +/// without them, or you know that you will be likely to. +/// +/// # Examples +/// +/// ```ignore +/// use nix::unistd::{Group, GroupQuery}; +/// // On many systems there's no `root` group; on FreeBSD for example it's known as `wheel` +/// let find = String::from("root"); +/// // Returns an Option>, thus the double unwrap. Will panic if there's no `root` +/// // group. +/// let res = Group::query( GroupQuery::Name(find.clone()) ).unwrap().unwrap(); +/// assert!(res.name == find); +/// ``` +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub enum GroupQuery { + /// Get a group by GID. + /// + /// Internally, this function calls + /// [getgrgid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + Gid(Gid), + /// Get a group by name. + /// + /// Internally, this function calls + /// [getgrnam_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + Name(String), + /// Get a group by GID, explicitly specifying the buffer size. + GidWithBufsize(Gid, usize), + /// Get a group by name, explicitly specifying the buffer size. + NameWithBufsize(String, usize) +} + +pub trait Queryable where Self: ::std::marker::Sized { + fn query(q: Q) -> Option>; +} + +/// Default buffer size for system user and group querying functions +const PWGRP_BUFSIZE: usize = 1024; + +impl Queryable for User { + fn query(q: UserQuery) -> Option> { + let mut cbuf = match q { + UserQuery::UidWithBufsize(_, size) | + UserQuery::NameWithBufsize(_, size) => vec![0 as c_char; size], + _ => vec![0 as c_char; PWGRP_BUFSIZE], + }; + + let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + let mut res = ptr::null_mut(); + + let error = { + unsafe { Errno::clear(); } + + match q { + UserQuery::UidWithBufsize(Uid(uid), _) | + UserQuery::Uid(Uid(uid)) => { + unsafe { + libc::getpwuid_r(uid, &mut pwd, + cbuf.as_mut_ptr(), + cbuf.len(), &mut res) + } + }, + UserQuery::NameWithBufsize(name, _) | + UserQuery::Name(name) => { + unsafe { + libc::getpwnam_r(CString::new(name).unwrap().as_ptr(), + &mut pwd, cbuf.as_mut_ptr(), cbuf.len(), &mut res) + } + }, + } + + }; + + if error == 0 { + if ! res.is_null() { + Some(Ok(User::from(res))) + } else { + None + } + } else { + Some(Err(Error::Sys(Errno::last()))) + } + } +} + +impl Queryable for Group { + fn query(q: GroupQuery) -> Option> { + let mut cbuf = match q { + GroupQuery::GidWithBufsize(_, size) | + GroupQuery::NameWithBufsize(_, size) => { + vec![0 as c_char; size] + }, + _ => { + vec![0 as c_char; PWGRP_BUFSIZE] + } + }; + + let mut grp: libc::group = unsafe { mem::uninitialized() }; + let mut res = ptr::null_mut(); + + let error = { + unsafe { Errno::clear(); } + + match q { + GroupQuery::GidWithBufsize(Gid(gid), _) | + GroupQuery::Gid(Gid(gid)) => { + unsafe { + libc::getgrgid_r(gid, &mut grp, + cbuf.as_mut_ptr(), + cbuf.len(), &mut res) + } + }, + GroupQuery::NameWithBufsize(name, _) | + GroupQuery::Name(name) => { + unsafe { + libc::getgrnam_r(CString::new(name).unwrap().as_ptr(), + &mut grp, cbuf.as_mut_ptr(), cbuf.len(), &mut res) + } + }, + } + + }; + + if error == 0 { + if !res.is_null() { + Some(Ok(Group::from(res))) + } else { + None + } + } else { + Some(Err(Error::Sys(Errno::last()))) + } + } +} + +#[cfg(not(any(target_os = "android", + target_os = "ios", + target_os = "macos", + target_env = "musl")))] +mod usergroupiter { + use libc::{self, c_char}; + use Result; + use errno::Errno; + use super::{Error, User, Group, PWGRP_BUFSIZE}; + use std::{mem, ptr}; + + /// Used to get all of the users on the system. + /// + /// # Examples + /// + /// ``` + /// # use nix::unistd::Users; + /// Users::new() + /// .map(|e|e.map( + /// |pw| println!("{}\t{}", + /// pw.name, + /// pw.uid))) + /// .collect::>(); + /// + /// ``` + /// + /// # Safety + /// + /// This iterator should not be used in different threads without synchronization; while doing so + /// will not cause undefined behavior, because modern systems lack re-entrant versions of + /// `setpwent` and `endpwent`, it is very likely that iterators running in different threads will + /// yield different numbers of items. + #[derive(Debug, Clone, Eq, PartialEq, Hash, Default)] + pub struct Users(usize); + + impl Users { + /// Create a new `Users` instance with default buffer size. + pub fn new() -> Self { + unsafe { libc::setpwent(); } + Users(PWGRP_BUFSIZE) + } + + /// Create a new `Users` instance with given buffer size. + pub fn with_bufsize(bufsize: usize) -> Self { + unsafe { libc::setpwent(); } + Users(bufsize) + } + + /// Get the buffer size this `Users` instance was created with. + pub fn bufsize(&self) -> usize { + self.0 + } + } + + impl Iterator for Users { + type Item = Result; + fn next(&mut self) -> Option> { + + let mut cbuf = vec![0 as c_char; self.0]; + let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + let mut res = ptr::null_mut(); + + let error = unsafe { + Errno::clear(); + libc::getpwent_r(&mut pwd, cbuf.as_mut_ptr(), self.0, &mut res) + }; + + if error == 0 && !res.is_null() { + Some(Ok(User::from(res))) + } else if error == libc::ERANGE { + Some(Err(Error::Sys(Errno::last()))) + } else { + None + } + } + } + + impl Drop for Users { + fn drop(&mut self) { + unsafe { libc::endpwent() }; + } + } + + /// Used to get all of the groups on the system. + /// + /// # Examples + /// + /// ``` + /// # use nix::unistd::Groups; + /// Groups::new() + /// .map(|e|e.map( + /// |gr| println!("{}\t{}", + /// gr.name, + /// gr.gid))) + /// .collect::>(); + /// ``` + /// + /// # Safety + /// + /// This iterator should not be used in different threads without synchronization; while doing so + /// will not cause undefined behavior, because modern systems lack re-entrant versions of + /// `setgrent` and `endgrent`, it is very likely that iterators running in different threads will + /// yield different numbers of items. + #[derive(Debug, Clone, Eq, PartialEq, Hash, Default)] + pub struct Groups(usize); + + impl Groups { + /// Create a new `Groups` instance with default buffer size. + pub fn new() -> Self { + unsafe { libc::setgrent(); } + Groups(PWGRP_BUFSIZE) + } + + /// Create a new `Groups` instance with given buffer size. + pub fn with_bufsize(bufsize: usize) -> Self { + unsafe { libc::setgrent(); } + Groups(bufsize) + } + + /// Get the buffer size this `Users` instance was created with. + pub fn bufsize(&self) -> usize { + self.0 + } + } + + impl Iterator for Groups { + type Item = Result; + fn next(&mut self) -> Option> { + + let mut cbuf = vec![0 as c_char; self.0]; + let mut grp: libc::group = unsafe { mem::zeroed() }; + let mut res = ptr::null_mut(); + + let error = unsafe { + Errno::clear(); + libc::getgrent_r(&mut grp, cbuf.as_mut_ptr(), self.0, &mut res) + }; + + if error == 0 && !res.is_null() { + Some(Ok(Group::from(res))) + } else if error == libc::ERANGE { + Some(Err(Error::Sys(Errno::last()))) + } else { + None + } + } + } + + impl Drop for Groups { + fn drop(&mut self) { + unsafe { libc::endgrent() }; + } + } +} diff --git a/test/test.rs b/test/test.rs index 6a71d261b5..b682a60485 100644 --- a/test/test.rs +++ b/test/test.rs @@ -120,6 +120,8 @@ lazy_static! { pub static ref PTSNAME_MTX: Mutex<()> = Mutex::new(()); /// Any test that alters signal handling must grab this mutex. pub static ref SIGNAL_MTX: Mutex<()> = Mutex::new(()); + /// Any test that uses the `Users` or `Groups` iterators must grab this mutex. + pub static ref USER_GRP_ITER_MTX: Mutex<()> = Mutex::new(()); } /// RAII object that restores a test's original directory on drop diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 46196dec7c..4f3e6ead74 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -600,6 +600,62 @@ fn test_symlinkat() { ); } +#[test] +fn test_getpwuid() { + let res = User::query( UserQuery::Uid(Uid::from_raw(1)) ).unwrap(); + assert!(res.unwrap().uid == Uid::from_raw(1)); +} + +#[test] +fn test_getgrgid() { + let res = Group::query( GroupQuery::Gid(Gid::from_raw(1)) ).unwrap(); + assert!(res.unwrap().gid == Gid::from_raw(1)); +} + +#[cfg(not(any(target_os = "android", + target_os = "ios", + target_os = "macos", + target_env = "musl")))] + +#[test] +fn test_users_iterator() { + let _m = ::USER_GRP_ITER_MTX.lock().expect("Mutex got poisoned by another test"); + + let entries = Users::new(); + let users: Vec> = entries.collect(); + let entries2 = Users::new(); + let users2: Vec> = entries2.collect(); + assert!(users == users2 && users.len() > 0); +} + +#[cfg(not(any(target_os = "android", + target_os = "ios", + target_os = "macos", + target_env = "musl")))] + +#[test] +fn test_groups_iterator() { + let _m = ::USER_GRP_ITER_MTX.lock().expect("Mutex got poisoned by another test"); + + let entries = Groups::new(); + let groups: Vec> = entries.collect(); + let entries2 = Groups::new(); + let groups2: Vec> = entries2.collect(); + assert!(groups == groups2 && groups.len() > 0); +} + +#[cfg(not(any(target_os = "android", + target_os = "ios", + target_os = "macos", + target_env = "musl")))] +#[test] +/// This test sees what happens when we use a ridiculously small buffer. +fn test_users_iterator_smallbuf() { + let _m = ::USER_GRP_ITER_MTX.lock().expect("Mutex got poisoned by another test"); + + let bufsize = 2; + assert!(Users::with_bufsize(bufsize).next().unwrap().is_err()); +} #[test] fn test_unlinkat_dir_noremovedir() { From 072c08d21017b0f29481be2ddd050e1b1c1b392b Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Mon, 19 Aug 2019 21:19:43 -0300 Subject: [PATCH 02/17] Initial rework of code; later will be squashed Signed-off-by: Otavio Salvador --- src/unistd.rs | 304 ++++++++++++++++++++------------------------ test/test_unistd.rs | 18 +-- 2 files changed, 149 insertions(+), 173 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index e8e4d2d05c..bf917cd882 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2398,11 +2398,14 @@ pub fn access(path: &P, amode: AccessFlags) -> Result<()> { Errno::result(res).map(drop) } +/// Default buffer size for system user and group querying functions +const PWGRP_BUFSIZE: usize = 1024; + /// Representation of a User, based on `libc::passwd` /// /// The reason some fields in this struct are `String` and others are `CString` is because some /// fields are based on the user's locale, which could be non-UTF8, while other fields are -/// guaranteed to conform to [NAME_REGEX](https://serverfault.com/a/73101/407341), which only +/// guaranteed to conform to [`NAME_REGEX`](https://serverfault.com/a/73101/407341), which only /// contains ASCII. #[derive(Debug, Clone, PartialEq, Default)] pub struct User { @@ -2455,6 +2458,79 @@ impl From<*mut libc::passwd> for User { } } +impl User { + /// Get a user by UID. + /// + /// Internally, this function calls + /// [getpwuid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + /// + /// # Examples + /// + /// ``` + /// use nix::unistd::{Uid, User}; + /// // Returns an Option>, thus the double unwrap. + /// let res = User::from_uid(Uid::from_raw(0), Some(2048)).unwrap().unwrap(); + /// assert!(res.name == "root"); + /// ``` + pub fn from_uid(uid: Uid, bufsize: Option) -> Option> { + let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; + let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + let mut res = ptr::null_mut(); + + let error = unsafe { + Errno::clear(); + libc::getpwuid_r(uid.0, &mut pwd, + cbuf.as_mut_ptr(), + cbuf.len(), &mut res) + }; + + if error == 0 { + if ! res.is_null() { + Some(Ok(User::from(res))) + } else { + None + } + } else { + Some(Err(Error::Sys(Errno::last()))) + } + } + + /// Get a user by name. + /// + /// Internally, this function calls + /// [getpwnam_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + /// + /// # Examples + /// + /// ``` + /// use nix::unistd::User; + /// // Returns an Option>, thus the double unwrap. + /// let res = User::from_name("root", Some(2048)).unwrap().unwrap(); + /// assert!(res.name == "root"); + /// ``` + pub fn from_name(name: &str, bufsize: Option) -> Option> { + let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; + let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + let mut res = ptr::null_mut(); + + let error = unsafe { + Errno::clear(); + libc::getpwnam_r(CString::new(name).unwrap().as_ptr(), + &mut pwd, cbuf.as_mut_ptr(), cbuf.len(), &mut res) + }; + + if error == 0 { + if ! res.is_null() { + Some(Ok(User::from(res))) + } else { + None + } + } else { + Some(Err(Error::Sys(Errno::last()))) + } + } +} + /// Representation of a Group, based on `libc::group` #[derive(Debug, Clone, PartialEq)] pub struct Group { @@ -2494,118 +2570,37 @@ impl Group { ret } -} - -/// Query a user. -/// -/// The buffer size variants should not be needed 99% of the time; the default buffer size of 1024 -/// should be more than adequate. Only use the buffer size variants if you receive an ERANGE error -/// without them, or you know that you will be likely to. -/// -/// # Examples -/// -/// ``` -/// use nix::unistd::{User, UserQuery, Queryable}; -/// let find = String::from("root"); -/// // Returns an Option>, thus the double unwrap. -/// let res = User::query( UserQuery::Name(find.clone()) ).unwrap().unwrap(); -/// assert!(res.name == find); -/// ``` -#[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub enum UserQuery { - /// Get a user by UID. - /// - /// Internally, this function calls - /// [getpwuid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) - Uid(Uid), - /// Get a user by name. - /// - /// Internally, this function calls - /// [getpwnam_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) - Name(String), - UidWithBufsize(Uid, usize), - NameWithBufsize(String, usize) -} -/// Query a group. -/// -/// The buffer size variants should not be needed 99% of the time; the default buffer size of 1024 -/// should be more than adequate. Only use the buffer size variants if you receive an ERANGE error -/// without them, or you know that you will be likely to. -/// -/// # Examples -/// -/// ```ignore -/// use nix::unistd::{Group, GroupQuery}; -/// // On many systems there's no `root` group; on FreeBSD for example it's known as `wheel` -/// let find = String::from("root"); -/// // Returns an Option>, thus the double unwrap. Will panic if there's no `root` -/// // group. -/// let res = Group::query( GroupQuery::Name(find.clone()) ).unwrap().unwrap(); -/// assert!(res.name == find); -/// ``` -#[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub enum GroupQuery { /// Get a group by GID. /// /// Internally, this function calls /// [getgrgid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) - Gid(Gid), - /// Get a group by name. /// - /// Internally, this function calls - /// [getgrnam_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) - Name(String), - /// Get a group by GID, explicitly specifying the buffer size. - GidWithBufsize(Gid, usize), - /// Get a group by name, explicitly specifying the buffer size. - NameWithBufsize(String, usize) -} - -pub trait Queryable where Self: ::std::marker::Sized { - fn query(q: Q) -> Option>; -} - -/// Default buffer size for system user and group querying functions -const PWGRP_BUFSIZE: usize = 1024; - -impl Queryable for User { - fn query(q: UserQuery) -> Option> { - let mut cbuf = match q { - UserQuery::UidWithBufsize(_, size) | - UserQuery::NameWithBufsize(_, size) => vec![0 as c_char; size], - _ => vec![0 as c_char; PWGRP_BUFSIZE], - }; - - let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + /// # Examples + /// + // Disable this test on all OS except Linux as root group may not exist. + #[cfg_attr(not(target_os = "linux"), doc = " ```no_run")] + #[cfg_attr(target_os = "linux", doc = " ```")] + /// use nix::unistd::{Gid, Group}; + /// // Returns an Option>, thus the double unwrap. + /// let res = Group::from_gid(Gid::from_raw(0), Some(2048)).unwrap().unwrap(); + /// assert!(res.name == "root"); + /// ``` + pub fn from_gid(gid: Gid, bufsize: Option) -> Option> { + let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; + let mut grp: libc::group = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); - let error = { - unsafe { Errno::clear(); } - - match q { - UserQuery::UidWithBufsize(Uid(uid), _) | - UserQuery::Uid(Uid(uid)) => { - unsafe { - libc::getpwuid_r(uid, &mut pwd, - cbuf.as_mut_ptr(), - cbuf.len(), &mut res) - } - }, - UserQuery::NameWithBufsize(name, _) | - UserQuery::Name(name) => { - unsafe { - libc::getpwnam_r(CString::new(name).unwrap().as_ptr(), - &mut pwd, cbuf.as_mut_ptr(), cbuf.len(), &mut res) - } - }, - } - + let error = unsafe { + Errno::clear(); + libc::getgrgid_r(gid.0, &mut grp, + cbuf.as_mut_ptr(), + cbuf.len(), &mut res) }; if error == 0 { - if ! res.is_null() { - Some(Ok(User::from(res))) + if !res.is_null() { + Some(Ok(Group::from(res))) } else { None } @@ -2613,44 +2608,31 @@ impl Queryable for User { Some(Err(Error::Sys(Errno::last()))) } } -} - -impl Queryable for Group { - fn query(q: GroupQuery) -> Option> { - let mut cbuf = match q { - GroupQuery::GidWithBufsize(_, size) | - GroupQuery::NameWithBufsize(_, size) => { - vec![0 as c_char; size] - }, - _ => { - vec![0 as c_char; PWGRP_BUFSIZE] - } - }; + /// Get a group by name. + /// + /// Internally, this function calls + /// [getgrnam_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + /// + /// # Examples + /// + // Disable this test on all OS except Linux as root group may not exist. + #[cfg_attr(not(target_os = "linux"), doc = " ```no_run")] + #[cfg_attr(target_os = "linux", doc = " ```")] + /// use nix::unistd::Group; + /// // Returns an Option>, thus the double unwrap. + /// let res = Group::from_name("root", Some(2048)).unwrap().unwrap(); + /// assert!(res.name == "root"); + /// ``` + pub fn from_name(name: &str, bufsize: Option) -> Option> { + let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; let mut grp: libc::group = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); - let error = { - unsafe { Errno::clear(); } - - match q { - GroupQuery::GidWithBufsize(Gid(gid), _) | - GroupQuery::Gid(Gid(gid)) => { - unsafe { - libc::getgrgid_r(gid, &mut grp, - cbuf.as_mut_ptr(), - cbuf.len(), &mut res) - } - }, - GroupQuery::NameWithBufsize(name, _) | - GroupQuery::Name(name) => { - unsafe { - libc::getgrnam_r(CString::new(name).unwrap().as_ptr(), - &mut grp, cbuf.as_mut_ptr(), cbuf.len(), &mut res) - } - }, - } - + let error = unsafe { + Errno::clear(); + libc::getgrnam_r(CString::new(name).unwrap().as_ptr(), + &mut grp, cbuf.as_mut_ptr(), cbuf.len(), &mut res) }; if error == 0 { @@ -2682,11 +2664,8 @@ mod usergroupiter { /// /// ``` /// # use nix::unistd::Users; - /// Users::new() - /// .map(|e|e.map( - /// |pw| println!("{}\t{}", - /// pw.name, - /// pw.uid))) + /// Users::default() + /// .map(|e| e.map(|pw| println!("{}\t{}", pw.name, pw.uid))) /// .collect::>(); /// /// ``` @@ -2697,18 +2676,18 @@ mod usergroupiter { /// will not cause undefined behavior, because modern systems lack re-entrant versions of /// `setpwent` and `endpwent`, it is very likely that iterators running in different threads will /// yield different numbers of items. - #[derive(Debug, Clone, Eq, PartialEq, Hash, Default)] + #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct Users(usize); - impl Users { - /// Create a new `Users` instance with default buffer size. - pub fn new() -> Self { - unsafe { libc::setpwent(); } - Users(PWGRP_BUFSIZE) + impl Default for Users { + fn default() -> Self { + Users::with_capacity(PWGRP_BUFSIZE) } + } - /// Create a new `Users` instance with given buffer size. - pub fn with_bufsize(bufsize: usize) -> Self { + impl Users { + /// Create a new `Users` instance with given capacity. + pub fn with_capacity(bufsize: usize) -> Self { unsafe { libc::setpwent(); } Users(bufsize) } @@ -2724,7 +2703,7 @@ mod usergroupiter { fn next(&mut self) -> Option> { let mut cbuf = vec![0 as c_char; self.0]; - let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + let mut pwd: libc::passwd = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); let error = unsafe { @@ -2754,11 +2733,8 @@ mod usergroupiter { /// /// ``` /// # use nix::unistd::Groups; - /// Groups::new() - /// .map(|e|e.map( - /// |gr| println!("{}\t{}", - /// gr.name, - /// gr.gid))) + /// Groups::default() + /// .map(|e| e.map(|gr| println!("{}\t{}", gr.name, gr.gid))) /// .collect::>(); /// ``` /// @@ -2768,18 +2744,18 @@ mod usergroupiter { /// will not cause undefined behavior, because modern systems lack re-entrant versions of /// `setgrent` and `endgrent`, it is very likely that iterators running in different threads will /// yield different numbers of items. - #[derive(Debug, Clone, Eq, PartialEq, Hash, Default)] + #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub struct Groups(usize); - impl Groups { - /// Create a new `Groups` instance with default buffer size. - pub fn new() -> Self { - unsafe { libc::setgrent(); } - Groups(PWGRP_BUFSIZE) + impl Default for Groups { + fn default() -> Self { + Groups::with_capacity(PWGRP_BUFSIZE) } + } - /// Create a new `Groups` instance with given buffer size. - pub fn with_bufsize(bufsize: usize) -> Self { + impl Groups { + /// Create a new `Groups` instance with given capacity. + pub fn with_capacity(bufsize: usize) -> Self { unsafe { libc::setgrent(); } Groups(bufsize) } @@ -2795,7 +2771,7 @@ mod usergroupiter { fn next(&mut self) -> Option> { let mut cbuf = vec![0 as c_char; self.0]; - let mut grp: libc::group = unsafe { mem::zeroed() }; + let mut grp: libc::group = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); let error = unsafe { diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 4f3e6ead74..c8dafccbe2 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -602,14 +602,14 @@ fn test_symlinkat() { #[test] fn test_getpwuid() { - let res = User::query( UserQuery::Uid(Uid::from_raw(1)) ).unwrap(); - assert!(res.unwrap().uid == Uid::from_raw(1)); + let res = User::from_uid(Uid::from_raw(0), None).unwrap(); + assert!(res.unwrap().uid == Uid::from_raw(0)); } #[test] fn test_getgrgid() { - let res = Group::query( GroupQuery::Gid(Gid::from_raw(1)) ).unwrap(); - assert!(res.unwrap().gid == Gid::from_raw(1)); + let res = Group::from_gid(Gid::from_raw(0), None).unwrap(); + assert!(res.unwrap().gid == Gid::from_raw(0)); } #[cfg(not(any(target_os = "android", @@ -621,9 +621,9 @@ fn test_getgrgid() { fn test_users_iterator() { let _m = ::USER_GRP_ITER_MTX.lock().expect("Mutex got poisoned by another test"); - let entries = Users::new(); + let entries = Users::default(); let users: Vec> = entries.collect(); - let entries2 = Users::new(); + let entries2 = Users::default(); let users2: Vec> = entries2.collect(); assert!(users == users2 && users.len() > 0); } @@ -637,9 +637,9 @@ fn test_users_iterator() { fn test_groups_iterator() { let _m = ::USER_GRP_ITER_MTX.lock().expect("Mutex got poisoned by another test"); - let entries = Groups::new(); + let entries = Groups::default(); let groups: Vec> = entries.collect(); - let entries2 = Groups::new(); + let entries2 = Groups::default(); let groups2: Vec> = entries2.collect(); assert!(groups == groups2 && groups.len() > 0); } @@ -654,7 +654,7 @@ fn test_users_iterator_smallbuf() { let _m = ::USER_GRP_ITER_MTX.lock().expect("Mutex got poisoned by another test"); let bufsize = 2; - assert!(Users::with_bufsize(bufsize).next().unwrap().is_err()); + assert!(Users::with_capacity(bufsize).next().unwrap().is_err()); } #[test] From 16f3cf5be404019c3a2ff329aebadb0057248a63 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Tue, 20 Aug 2019 19:57:21 -0300 Subject: [PATCH 03/17] Reduce initialization Signed-off-by: Otavio Salvador --- src/unistd.rs | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index bf917cd882..a3b5c0a46a 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2473,15 +2473,14 @@ impl User { /// assert!(res.name == "root"); /// ``` pub fn from_uid(uid: Uid, bufsize: Option) -> Option> { - let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; - let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); + let mut pwd: libc::passwd = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getpwuid_r(uid.0, &mut pwd, - cbuf.as_mut_ptr(), - cbuf.len(), &mut res) + libc::getpwuid_r(uid.0, &mut pwd, cbuf.as_mut_ptr(), + cbuf.capacity(), &mut res) }; if error == 0 { @@ -2509,14 +2508,14 @@ impl User { /// assert!(res.name == "root"); /// ``` pub fn from_name(name: &str, bufsize: Option) -> Option> { - let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; - let mut pwd: libc::passwd = unsafe { mem::zeroed() }; + let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); + let mut pwd: libc::passwd = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getpwnam_r(CString::new(name).unwrap().as_ptr(), - &mut pwd, cbuf.as_mut_ptr(), cbuf.len(), &mut res) + libc::getpwnam_r(CString::new(name).unwrap().as_ptr(), &mut pwd, + cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) }; if error == 0 { @@ -2556,7 +2555,7 @@ impl From<*mut libc::group> for Group { impl Group { unsafe fn members(mem: *mut *mut c_char) -> Vec { - let mut ret = vec![]; + let mut ret = Vec::new(); for i in 0.. { let u = mem.offset(i); @@ -2587,15 +2586,14 @@ impl Group { /// assert!(res.name == "root"); /// ``` pub fn from_gid(gid: Gid, bufsize: Option) -> Option> { - let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; + let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); let mut grp: libc::group = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getgrgid_r(gid.0, &mut grp, - cbuf.as_mut_ptr(), - cbuf.len(), &mut res) + libc::getgrgid_r(gid.0, &mut grp, cbuf.as_mut_ptr(), + cbuf.capacity(), &mut res) }; if error == 0 { @@ -2625,14 +2623,14 @@ impl Group { /// assert!(res.name == "root"); /// ``` pub fn from_name(name: &str, bufsize: Option) -> Option> { - let mut cbuf = vec![0 as c_char; bufsize.unwrap_or(PWGRP_BUFSIZE)]; + let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); let mut grp: libc::group = unsafe { mem::uninitialized() }; let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getgrnam_r(CString::new(name).unwrap().as_ptr(), - &mut grp, cbuf.as_mut_ptr(), cbuf.len(), &mut res) + libc::getgrnam_r(CString::new(name).unwrap().as_ptr(), &mut grp, + cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) }; if error == 0 { From a8143fb6b2c52b48478a681dae6f740c105bede5 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Tue, 20 Aug 2019 21:55:57 -0300 Subject: [PATCH 04/17] Use MaybeUninit Signed-off-by: Otavio Salvador --- src/unistd.rs | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index a3b5c0a46a..43fe149743 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2474,13 +2474,18 @@ impl User { /// ``` pub fn from_uid(uid: Uid, bufsize: Option) -> Option> { let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); - let mut pwd: libc::passwd = unsafe { mem::uninitialized() }; + let mut pwd = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getpwuid_r(uid.0, &mut pwd, cbuf.as_mut_ptr(), - cbuf.capacity(), &mut res) + libc::getpwuid_r( + uid.0, + pwd.as_mut_ptr(), + cbuf.as_mut_ptr(), + cbuf.capacity(), + &mut res + ) }; if error == 0 { @@ -2509,13 +2514,18 @@ impl User { /// ``` pub fn from_name(name: &str, bufsize: Option) -> Option> { let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); - let mut pwd: libc::passwd = unsafe { mem::uninitialized() }; + let mut pwd = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getpwnam_r(CString::new(name).unwrap().as_ptr(), &mut pwd, - cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) + libc::getpwnam_r( + CString::new(name).unwrap().as_ptr(), + pwd.as_mut_ptr(), + cbuf.as_mut_ptr(), + cbuf.capacity(), + &mut res + ) }; if error == 0 { @@ -2587,13 +2597,18 @@ impl Group { /// ``` pub fn from_gid(gid: Gid, bufsize: Option) -> Option> { let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); - let mut grp: libc::group = unsafe { mem::uninitialized() }; + let mut grp = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getgrgid_r(gid.0, &mut grp, cbuf.as_mut_ptr(), - cbuf.capacity(), &mut res) + libc::getgrgid_r( + gid.0, + grp.as_mut_ptr(), + cbuf.as_mut_ptr(), + cbuf.capacity(), + &mut res + ) }; if error == 0 { @@ -2624,13 +2639,18 @@ impl Group { /// ``` pub fn from_name(name: &str, bufsize: Option) -> Option> { let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); - let mut grp: libc::group = unsafe { mem::uninitialized() }; + let mut grp = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getgrnam_r(CString::new(name).unwrap().as_ptr(), &mut grp, - cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) + libc::getgrnam_r( + CString::new(name).unwrap().as_ptr(), + grp.as_mut_ptr(), + cbuf.as_mut_ptr(), + cbuf.capacity(), + &mut res + ) }; if error == 0 { From de1ed63a258ad62eded65af92cd6e1d6bea05552 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Thu, 22 Aug 2019 00:59:51 -0300 Subject: [PATCH 05/17] Avoid using From<*mut libc::...> using reference Signed-off-by: Otavio Salvador --- src/unistd.rs | 52 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index 43fe149743..514dc366a2 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2435,8 +2435,8 @@ pub struct User { pub expire: libc::time_t } -impl From<*mut libc::passwd> for User { - fn from(pw: *mut libc::passwd) -> User { +impl From<&libc::passwd> for User { + fn from(pw: &libc::passwd) -> User { unsafe { User { name: CStr::from_ptr((*pw).pw_name).to_string_lossy().into_owned(), @@ -2488,9 +2488,11 @@ impl User { ) }; + let pwd = unsafe { pwd.assume_init() }; + if error == 0 { if ! res.is_null() { - Some(Ok(User::from(res))) + Some(Ok(User::from(&pwd))) } else { None } @@ -2528,9 +2530,11 @@ impl User { ) }; + let pwd = unsafe { pwd.assume_init() }; + if error == 0 { if ! res.is_null() { - Some(Ok(User::from(res))) + Some(Ok(User::from(&pwd))) } else { None } @@ -2551,8 +2555,8 @@ pub struct Group { pub mem: Vec } -impl From<*mut libc::group> for Group { - fn from(gr: *mut libc::group) -> Group { +impl From<&libc::group> for Group { + fn from(gr: &libc::group) -> Group { unsafe { Group { name: CStr::from_ptr((*gr).gr_name).to_string_lossy().into_owned(), @@ -2611,9 +2615,11 @@ impl Group { ) }; + let grp = unsafe { grp.assume_init() }; + if error == 0 { if !res.is_null() { - Some(Ok(Group::from(res))) + Some(Ok(Group::from(&grp))) } else { None } @@ -2653,9 +2659,11 @@ impl Group { ) }; + let grp = unsafe { grp.assume_init() }; + if error == 0 { if !res.is_null() { - Some(Ok(Group::from(res))) + Some(Ok(Group::from(&grp))) } else { None } @@ -2719,18 +2727,24 @@ mod usergroupiter { impl Iterator for Users { type Item = Result; fn next(&mut self) -> Option> { - let mut cbuf = vec![0 as c_char; self.0]; - let mut pwd: libc::passwd = unsafe { mem::uninitialized() }; + let mut pwd = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getpwent_r(&mut pwd, cbuf.as_mut_ptr(), self.0, &mut res) + libc::getpwent_r( + pwd.as_mut_ptr(), + cbuf.as_mut_ptr(), + self.0, + &mut res + ) }; + let pwd = unsafe { pwd.assume_init() }; + if error == 0 && !res.is_null() { - Some(Ok(User::from(res))) + Some(Ok(User::from(&pwd))) } else if error == libc::ERANGE { Some(Err(Error::Sys(Errno::last()))) } else { @@ -2787,18 +2801,24 @@ mod usergroupiter { impl Iterator for Groups { type Item = Result; fn next(&mut self) -> Option> { - let mut cbuf = vec![0 as c_char; self.0]; - let mut grp: libc::group = unsafe { mem::uninitialized() }; + let mut grp = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getgrent_r(&mut grp, cbuf.as_mut_ptr(), self.0, &mut res) + libc::getgrent_r( + grp.as_mut_ptr(), + cbuf.as_mut_ptr(), + self.0, + &mut res + ) }; + let grp = unsafe { grp.assume_init() }; + if error == 0 && !res.is_null() { - Some(Ok(Group::from(res))) + Some(Ok(Group::from(&grp))) } else if error == libc::ERANGE { Some(Err(Error::Sys(Errno::last()))) } else { From 51d39cf469487b7c5c9f29bd5f35b54667aa7542 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Thu, 22 Aug 2019 02:46:57 -0300 Subject: [PATCH 06/17] Reduce code duplication Signed-off-by: Otavio Salvador --- src/unistd.rs | 170 ++++++++++++++++++-------------------------- test/test_unistd.rs | 12 ---- 2 files changed, 68 insertions(+), 114 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index 514dc366a2..6c96f0025e 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2459,33 +2459,19 @@ impl From<&libc::passwd> for User { } impl User { - /// Get a user by UID. - /// - /// Internally, this function calls - /// [getpwuid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) - /// - /// # Examples - /// - /// ``` - /// use nix::unistd::{Uid, User}; - /// // Returns an Option>, thus the double unwrap. - /// let res = User::from_uid(Uid::from_raw(0), Some(2048)).unwrap().unwrap(); - /// assert!(res.name == "root"); - /// ``` - pub fn from_uid(uid: Uid, bufsize: Option) -> Option> { - let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); + fn from_anything(f: impl Fn(*mut libc::passwd, + *mut libc::c_char, + libc::size_t, + *mut *mut libc::passwd) -> libc::c_int) + -> Option> + { + let mut cbuf = Vec::with_capacity(PWGRP_BUFSIZE); let mut pwd = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getpwuid_r( - uid.0, - pwd.as_mut_ptr(), - cbuf.as_mut_ptr(), - cbuf.capacity(), - &mut res - ) + f(pwd.as_mut_ptr(), cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) }; let pwd = unsafe { pwd.assume_init() }; @@ -2500,6 +2486,25 @@ impl User { Some(Err(Error::Sys(Errno::last()))) } } + + /// Get a user by UID. + /// + /// Internally, this function calls + /// [getpwuid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + /// + /// # Examples + /// + /// ``` + /// use nix::unistd::{Uid, User}; + /// // Returns an Option>, thus the double unwrap. + /// let res = User::from_uid(Uid::from_raw(0)).unwrap().unwrap(); + /// assert!(res.name == "root"); + /// ``` + pub fn from_uid(uid: Uid) -> Option> { + User::from_anything(|pwd, cbuf, cap, res| { + unsafe { libc::getpwuid_r(uid.0, pwd, cbuf, cap, res) } + }) + } /// Get a user by name. /// @@ -2511,36 +2516,14 @@ impl User { /// ``` /// use nix::unistd::User; /// // Returns an Option>, thus the double unwrap. - /// let res = User::from_name("root", Some(2048)).unwrap().unwrap(); + /// let res = User::from_name("root").unwrap().unwrap(); /// assert!(res.name == "root"); /// ``` - pub fn from_name(name: &str, bufsize: Option) -> Option> { - let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); - let mut pwd = mem::MaybeUninit::::uninit(); - let mut res = ptr::null_mut(); - - let error = unsafe { - Errno::clear(); - libc::getpwnam_r( - CString::new(name).unwrap().as_ptr(), - pwd.as_mut_ptr(), - cbuf.as_mut_ptr(), - cbuf.capacity(), - &mut res - ) - }; - - let pwd = unsafe { pwd.assume_init() }; - - if error == 0 { - if ! res.is_null() { - Some(Ok(User::from(&pwd))) - } else { - None - } - } else { - Some(Err(Error::Sys(Errno::last()))) - } + pub fn from_name(name: &str) -> Option> { + let name = CString::new(name).unwrap(); + User::from_anything(|pwd, cbuf, cap, res| { + unsafe { libc::getpwnam_r(name.as_ptr(), pwd, cbuf, cap, res) } + }) } } @@ -2584,35 +2567,19 @@ impl Group { ret } - /// Get a group by GID. - /// - /// Internally, this function calls - /// [getgrgid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) - /// - /// # Examples - /// - // Disable this test on all OS except Linux as root group may not exist. - #[cfg_attr(not(target_os = "linux"), doc = " ```no_run")] - #[cfg_attr(target_os = "linux", doc = " ```")] - /// use nix::unistd::{Gid, Group}; - /// // Returns an Option>, thus the double unwrap. - /// let res = Group::from_gid(Gid::from_raw(0), Some(2048)).unwrap().unwrap(); - /// assert!(res.name == "root"); - /// ``` - pub fn from_gid(gid: Gid, bufsize: Option) -> Option> { - let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); + fn from_anything(f: impl Fn(*mut libc::group, + *mut libc::c_char, + libc::size_t, + *mut *mut libc::group) -> libc::c_int) + -> Option> + { + let mut cbuf = Vec::with_capacity(PWGRP_BUFSIZE); let mut grp = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); let error = unsafe { Errno::clear(); - libc::getgrgid_r( - gid.0, - grp.as_mut_ptr(), - cbuf.as_mut_ptr(), - cbuf.capacity(), - &mut res - ) + f(grp.as_mut_ptr(), cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) }; let grp = unsafe { grp.assume_init() }; @@ -2628,6 +2595,27 @@ impl Group { } } + /// Get a group by GID. + /// + /// Internally, this function calls + /// [getgrgid_r(3)](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid_r.html) + /// + /// # Examples + /// + // Disable this test on all OS except Linux as root group may not exist. + #[cfg_attr(not(target_os = "linux"), doc = " ```no_run")] + #[cfg_attr(target_os = "linux", doc = " ```")] + /// use nix::unistd::{Gid, Group}; + /// // Returns an Option>, thus the double unwrap. + /// let res = Group::from_gid(Gid::from_raw(0)).unwrap().unwrap(); + /// assert!(res.name == "root"); + /// ``` + pub fn from_gid(gid: Gid) -> Option> { + Group::from_anything(|grp, cbuf, cap, res| { + unsafe { libc::getgrgid_r(gid.0, grp, cbuf, cap, res) } + }) + } + /// Get a group by name. /// /// Internally, this function calls @@ -2640,36 +2628,14 @@ impl Group { #[cfg_attr(target_os = "linux", doc = " ```")] /// use nix::unistd::Group; /// // Returns an Option>, thus the double unwrap. - /// let res = Group::from_name("root", Some(2048)).unwrap().unwrap(); + /// let res = Group::from_name("root").unwrap().unwrap(); /// assert!(res.name == "root"); /// ``` - pub fn from_name(name: &str, bufsize: Option) -> Option> { - let mut cbuf = Vec::with_capacity(bufsize.unwrap_or(PWGRP_BUFSIZE)); - let mut grp = mem::MaybeUninit::::uninit(); - let mut res = ptr::null_mut(); - - let error = unsafe { - Errno::clear(); - libc::getgrnam_r( - CString::new(name).unwrap().as_ptr(), - grp.as_mut_ptr(), - cbuf.as_mut_ptr(), - cbuf.capacity(), - &mut res - ) - }; - - let grp = unsafe { grp.assume_init() }; - - if error == 0 { - if !res.is_null() { - Some(Ok(Group::from(&grp))) - } else { - None - } - } else { - Some(Err(Error::Sys(Errno::last()))) - } + pub fn from_name(name: &str) -> Option> { + let name = CString::new(name).unwrap(); + Group::from_anything(|grp, cbuf, cap, res| { + unsafe { libc::getgrnam_r(name.as_ptr(), grp, cbuf, cap, res) } + }) } } diff --git a/test/test_unistd.rs b/test/test_unistd.rs index c8dafccbe2..5d29ebcaab 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -600,18 +600,6 @@ fn test_symlinkat() { ); } -#[test] -fn test_getpwuid() { - let res = User::from_uid(Uid::from_raw(0), None).unwrap(); - assert!(res.unwrap().uid == Uid::from_raw(0)); -} - -#[test] -fn test_getgrgid() { - let res = Group::from_gid(Gid::from_raw(0), None).unwrap(); - assert!(res.unwrap().gid == Gid::from_raw(0)); -} - #[cfg(not(any(target_os = "android", target_os = "ios", target_os = "macos", From 3b754392dfb14efbb095b3bdae365a5fbb4e80b5 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Thu, 22 Aug 2019 03:40:23 -0300 Subject: [PATCH 07/17] Handle ERANGE Signed-off-by: Otavio Salvador --- src/unistd.rs | 74 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 48 insertions(+), 26 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index 6c96f0025e..e11f537e07 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2465,25 +2465,36 @@ impl User { *mut *mut libc::passwd) -> libc::c_int) -> Option> { - let mut cbuf = Vec::with_capacity(PWGRP_BUFSIZE); + let bufsize = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { + Ok(Some(n)) => n as usize, + Ok(None) | Err(_) => 1024 as usize, + }; + + let mut cbuf = Vec::with_capacity(bufsize); let mut pwd = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); - let error = unsafe { - Errno::clear(); - f(pwd.as_mut_ptr(), cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) - }; - - let pwd = unsafe { pwd.assume_init() }; + loop { + let error = unsafe { + Errno::clear(); + f(pwd.as_mut_ptr(), cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) + }; - if error == 0 { - if ! res.is_null() { - Some(Ok(User::from(&pwd))) + if error == 0 { + if res.is_null() { + return None; + } else { + let pwd = unsafe { pwd.assume_init() }; + return Some(Ok(User::from(&pwd))); + } + } else if Errno::last() == Errno::ERANGE { + // Trigger the internal buffer resizing logic of `Vec` by requiring + // more space than the current capacity. + unsafe { cbuf.set_len(cbuf.capacity()); } + cbuf.reserve(1); } else { - None + return Some(Err(Error::Sys(Errno::last()))); } - } else { - Some(Err(Error::Sys(Errno::last()))) } } @@ -2573,25 +2584,36 @@ impl Group { *mut *mut libc::group) -> libc::c_int) -> Option> { - let mut cbuf = Vec::with_capacity(PWGRP_BUFSIZE); + let bufsize = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { + Ok(Some(n)) => n as usize, + Ok(None) | Err(_) => 1024 as usize, + }; + + let mut cbuf = Vec::with_capacity(bufsize); let mut grp = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); - let error = unsafe { - Errno::clear(); - f(grp.as_mut_ptr(), cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) - }; - - let grp = unsafe { grp.assume_init() }; + loop { + let error = unsafe { + Errno::clear(); + f(grp.as_mut_ptr(), cbuf.as_mut_ptr(), cbuf.capacity(), &mut res) + }; - if error == 0 { - if !res.is_null() { - Some(Ok(Group::from(&grp))) + if error == 0 { + if res.is_null() { + return None; + } else { + let grp = unsafe { grp.assume_init() }; + return Some(Ok(Group::from(&grp))); + } + } else if Errno::last() == Errno::ERANGE { + // Trigger the internal buffer resizing logic of `Vec` by requiring + // more space than the current capacity. + unsafe { cbuf.set_len(cbuf.capacity()); } + cbuf.reserve(1); } else { - None + return Some(Err(Error::Sys(Errno::last()))); } - } else { - Some(Err(Error::Sys(Errno::last()))) } } From b3d7716cefe17edf4fabebc4fec952080eedc42f Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Thu, 22 Aug 2019 03:50:21 -0300 Subject: [PATCH 08/17] Use Result> so it is more ergonimic Signed-off-by: Otavio Salvador --- src/unistd.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index e11f537e07..7381dc3612 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2463,7 +2463,7 @@ impl User { *mut libc::c_char, libc::size_t, *mut *mut libc::passwd) -> libc::c_int) - -> Option> + -> Result> { let bufsize = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { Ok(Some(n)) => n as usize, @@ -2482,10 +2482,10 @@ impl User { if error == 0 { if res.is_null() { - return None; + return Ok(None); } else { let pwd = unsafe { pwd.assume_init() }; - return Some(Ok(User::from(&pwd))); + return Ok(Some(User::from(&pwd))); } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic of `Vec` by requiring @@ -2493,7 +2493,7 @@ impl User { unsafe { cbuf.set_len(cbuf.capacity()); } cbuf.reserve(1); } else { - return Some(Err(Error::Sys(Errno::last()))); + return Err(Error::Sys(Errno::last())); } } } @@ -2507,11 +2507,11 @@ impl User { /// /// ``` /// use nix::unistd::{Uid, User}; - /// // Returns an Option>, thus the double unwrap. + /// // Returns an Result>, thus the double unwrap. /// let res = User::from_uid(Uid::from_raw(0)).unwrap().unwrap(); /// assert!(res.name == "root"); /// ``` - pub fn from_uid(uid: Uid) -> Option> { + pub fn from_uid(uid: Uid) -> Result> { User::from_anything(|pwd, cbuf, cap, res| { unsafe { libc::getpwuid_r(uid.0, pwd, cbuf, cap, res) } }) @@ -2526,11 +2526,11 @@ impl User { /// /// ``` /// use nix::unistd::User; - /// // Returns an Option>, thus the double unwrap. + /// // Returns an Result>, thus the double unwrap. /// let res = User::from_name("root").unwrap().unwrap(); /// assert!(res.name == "root"); /// ``` - pub fn from_name(name: &str) -> Option> { + pub fn from_name(name: &str) -> Result> { let name = CString::new(name).unwrap(); User::from_anything(|pwd, cbuf, cap, res| { unsafe { libc::getpwnam_r(name.as_ptr(), pwd, cbuf, cap, res) } @@ -2582,7 +2582,7 @@ impl Group { *mut libc::c_char, libc::size_t, *mut *mut libc::group) -> libc::c_int) - -> Option> + -> Result> { let bufsize = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { Ok(Some(n)) => n as usize, @@ -2601,10 +2601,10 @@ impl Group { if error == 0 { if res.is_null() { - return None; + return Ok(None); } else { let grp = unsafe { grp.assume_init() }; - return Some(Ok(Group::from(&grp))); + return Ok(Some(Group::from(&grp))); } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic of `Vec` by requiring @@ -2612,7 +2612,7 @@ impl Group { unsafe { cbuf.set_len(cbuf.capacity()); } cbuf.reserve(1); } else { - return Some(Err(Error::Sys(Errno::last()))); + return Err(Error::Sys(Errno::last())); } } } @@ -2628,11 +2628,11 @@ impl Group { #[cfg_attr(not(target_os = "linux"), doc = " ```no_run")] #[cfg_attr(target_os = "linux", doc = " ```")] /// use nix::unistd::{Gid, Group}; - /// // Returns an Option>, thus the double unwrap. + /// // Returns an Result>, thus the double unwrap. /// let res = Group::from_gid(Gid::from_raw(0)).unwrap().unwrap(); /// assert!(res.name == "root"); /// ``` - pub fn from_gid(gid: Gid) -> Option> { + pub fn from_gid(gid: Gid) -> Result> { Group::from_anything(|grp, cbuf, cap, res| { unsafe { libc::getgrgid_r(gid.0, grp, cbuf, cap, res) } }) @@ -2649,11 +2649,11 @@ impl Group { #[cfg_attr(not(target_os = "linux"), doc = " ```no_run")] #[cfg_attr(target_os = "linux", doc = " ```")] /// use nix::unistd::Group; - /// // Returns an Option>, thus the double unwrap. + /// // Returns an Result>, thus the double unwrap. /// let res = Group::from_name("root").unwrap().unwrap(); /// assert!(res.name == "root"); /// ``` - pub fn from_name(name: &str) -> Option> { + pub fn from_name(name: &str) -> Result> { let name = CString::new(name).unwrap(); Group::from_anything(|grp, cbuf, cap, res| { unsafe { libc::getgrnam_r(name.as_ptr(), grp, cbuf, cap, res) } From b361db0e86e00b6b43bd874708dab0aec3e619f0 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 28 Aug 2019 17:56:51 -0300 Subject: [PATCH 09/17] Improve `from_anything` definition using where clause Signed-off-by: Otavio Salvador --- src/unistd.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index 7381dc3612..af5990a308 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2459,11 +2459,12 @@ impl From<&libc::passwd> for User { } impl User { - fn from_anything(f: impl Fn(*mut libc::passwd, - *mut libc::c_char, - libc::size_t, - *mut *mut libc::passwd) -> libc::c_int) - -> Result> + fn from_anything(f: F) -> Result> + where + F: Fn(*mut libc::passwd, + *mut libc::c_char, + libc::size_t, + *mut *mut libc::passwd) -> libc::c_int { let bufsize = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { Ok(Some(n)) => n as usize, @@ -2578,11 +2579,12 @@ impl Group { ret } - fn from_anything(f: impl Fn(*mut libc::group, - *mut libc::c_char, - libc::size_t, - *mut *mut libc::group) -> libc::c_int) - -> Result> + fn from_anything(f: F) -> Result> + where + F: Fn(*mut libc::group, + *mut libc::c_char, + libc::size_t, + *mut *mut libc::group) -> libc::c_int { let bufsize = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { Ok(Some(n)) => n as usize, From 76e33fe8e729544f2ef7422fac3d999e8b148d21 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 4 Sep 2019 09:41:18 -0300 Subject: [PATCH 10/17] Add a generic reserve_buffer_size method Signed-off-by: Otavio Salvador --- src/unistd.rs | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index af5990a308..d245bcce37 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -5,7 +5,7 @@ use {Error, Result, NixPath}; use fcntl::{AtFlags, at_rawfd, fcntl, FdFlag, OFlag}; use fcntl::FcntlArg::F_SETFD; use libc::{self, c_char, c_void, c_int, c_long, c_uint, size_t, pid_t, off_t, - uid_t, gid_t, mode_t}; + uid_t, gid_t, mode_t, PATH_MAX}; use std::{fmt, mem, ptr}; use std::ffi::{CString, CStr, OsString, OsStr}; use std::os::unix::ffi::{OsStringExt, OsStrExt}; @@ -539,6 +539,23 @@ pub fn symlinkat( Errno::result(res).map(drop) } +// Double the buffer capacity up to limit. In case it already has +// reached the limit, return Errno::ERANGE. +fn reserve_buffer_size(buf: &mut Vec, limit: usize) -> Result<()> { + use std::cmp::min; + + if buf.len() == limit { + return Err(Error::Sys(Errno::ERANGE)) + } + + unsafe { buf.set_len(buf.capacity()) }; + + let capacity = min(buf.capacity() * 2, limit); + buf.reserve_exact(capacity); + + Ok(()) +} + /// Returns the current directory as a `PathBuf` /// /// Err is returned if the current user doesn't have the permission to read or search a component @@ -581,11 +598,8 @@ pub fn getcwd() -> Result { } } - // Trigger the internal buffer resizing logic of `Vec` by requiring - // more space than the current capacity. - let cap = buf.capacity(); - buf.set_len(cap); - buf.reserve(1); + // Trigger the internal buffer resizing logic. + reserve_buffer_size(&mut buf, PATH_MAX as usize)?; } } } @@ -2466,12 +2480,12 @@ impl User { libc::size_t, *mut *mut libc::passwd) -> libc::c_int { - let bufsize = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { + let cbuf_max = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { Ok(Some(n)) => n as usize, Ok(None) | Err(_) => 1024 as usize, }; - let mut cbuf = Vec::with_capacity(bufsize); + let mut cbuf = Vec::with_capacity(512); let mut pwd = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); @@ -2489,10 +2503,8 @@ impl User { return Ok(Some(User::from(&pwd))); } } else if Errno::last() == Errno::ERANGE { - // Trigger the internal buffer resizing logic of `Vec` by requiring - // more space than the current capacity. - unsafe { cbuf.set_len(cbuf.capacity()); } - cbuf.reserve(1); + // Trigger the internal buffer resizing logic. + reserve_buffer_size(&mut cbuf, cbuf_max)?; } else { return Err(Error::Sys(Errno::last())); } @@ -2586,12 +2598,12 @@ impl Group { libc::size_t, *mut *mut libc::group) -> libc::c_int { - let bufsize = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { + let cbuf_max = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { Ok(Some(n)) => n as usize, Ok(None) | Err(_) => 1024 as usize, }; - let mut cbuf = Vec::with_capacity(bufsize); + let mut cbuf = Vec::with_capacity(512); let mut grp = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); @@ -2609,10 +2621,8 @@ impl Group { return Ok(Some(Group::from(&grp))); } } else if Errno::last() == Errno::ERANGE { - // Trigger the internal buffer resizing logic of `Vec` by requiring - // more space than the current capacity. - unsafe { cbuf.set_len(cbuf.capacity()); } - cbuf.reserve(1); + // Trigger the internal buffer resizing logic. + reserve_buffer_size(&mut cbuf, cbuf_max)?; } else { return Err(Error::Sys(Errno::last())); } From 5eda92f189f96d954db0bacaa792cffd11baf69e Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 4 Sep 2019 22:26:20 -0300 Subject: [PATCH 11/17] Rework generic reserve_buffer_size code Signed-off-by: Otavio Salvador --- src/unistd.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index d245bcce37..cef01d9e22 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -544,14 +544,12 @@ pub fn symlinkat( fn reserve_buffer_size(buf: &mut Vec, limit: usize) -> Result<()> { use std::cmp::min; - if buf.len() == limit { + if buf.len() >= limit { return Err(Error::Sys(Errno::ERANGE)) } - unsafe { buf.set_len(buf.capacity()) }; - let capacity = min(buf.capacity() * 2, limit); - buf.reserve_exact(capacity); + buf.reserve(capacity); Ok(()) } @@ -2480,12 +2478,12 @@ impl User { libc::size_t, *mut *mut libc::passwd) -> libc::c_int { - let cbuf_max = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { + let bufsize = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { Ok(Some(n)) => n as usize, Ok(None) | Err(_) => 1024 as usize, }; - let mut cbuf = Vec::with_capacity(512); + let mut cbuf = Vec::with_capacity(bufsize); let mut pwd = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); @@ -2504,7 +2502,7 @@ impl User { } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic. - reserve_buffer_size(&mut cbuf, cbuf_max)?; + reserve_buffer_size(&mut cbuf, bufsize)?; } else { return Err(Error::Sys(Errno::last())); } @@ -2598,12 +2596,12 @@ impl Group { libc::size_t, *mut *mut libc::group) -> libc::c_int { - let cbuf_max = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { + let bufsize = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { Ok(Some(n)) => n as usize, Ok(None) | Err(_) => 1024 as usize, }; - let mut cbuf = Vec::with_capacity(512); + let mut cbuf = Vec::with_capacity(bufsize); let mut grp = mem::MaybeUninit::::uninit(); let mut res = ptr::null_mut(); @@ -2622,7 +2620,7 @@ impl Group { } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic. - reserve_buffer_size(&mut cbuf, cbuf_max)?; + reserve_buffer_size(&mut cbuf, bufsize)?; } else { return Err(Error::Sys(Errno::last())); } From d0b23f3a1075e61d6212980195aa03fdc93360cf Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 4 Sep 2019 22:33:54 -0300 Subject: [PATCH 12/17] Use reserve_double_buffer_size as method name Signed-off-by: Otavio Salvador --- src/unistd.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index cef01d9e22..329b6bdc0a 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -541,7 +541,7 @@ pub fn symlinkat( // Double the buffer capacity up to limit. In case it already has // reached the limit, return Errno::ERANGE. -fn reserve_buffer_size(buf: &mut Vec, limit: usize) -> Result<()> { +fn reserve_double_buffer_size(buf: &mut Vec, limit: usize) -> Result<()> { use std::cmp::min; if buf.len() >= limit { @@ -597,7 +597,7 @@ pub fn getcwd() -> Result { } // Trigger the internal buffer resizing logic. - reserve_buffer_size(&mut buf, PATH_MAX as usize)?; + reserve_double_buffer_size(&mut buf, PATH_MAX as usize)?; } } } @@ -2502,7 +2502,7 @@ impl User { } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic. - reserve_buffer_size(&mut cbuf, bufsize)?; + reserve_double_buffer_size(&mut cbuf, bufsize)?; } else { return Err(Error::Sys(Errno::last())); } @@ -2620,7 +2620,7 @@ impl Group { } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic. - reserve_buffer_size(&mut cbuf, bufsize)?; + reserve_double_buffer_size(&mut cbuf, bufsize)?; } else { return Err(Error::Sys(Errno::last())); } From 56b0cd1d887d012815ff26dd48049475aa42d5e7 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 4 Sep 2019 22:53:38 -0300 Subject: [PATCH 13/17] Fix unused code warnings Signed-off-by: Otavio Salvador --- src/unistd.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/unistd.rs b/src/unistd.rs index 329b6bdc0a..d64aef4ae5 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2410,6 +2410,10 @@ pub fn access(path: &P, amode: AccessFlags) -> Result<()> { Errno::result(res).map(drop) } +#[cfg(not(any(target_os = "android", + target_os = "ios", + target_os = "macos", + target_env = "musl")))] /// Default buffer size for system user and group querying functions const PWGRP_BUFSIZE: usize = 1024; From cc9f81c40fb9ec871b28dfb5ce8b37124385df29 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 11 Sep 2019 00:50:40 -0300 Subject: [PATCH 14/17] Drop Default from User as this should be inquired Signed-off-by: Otavio Salvador --- src/unistd.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index d64aef4ae5..e9b1933879 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -30,7 +30,7 @@ pub use self::usergroupiter::*; /// /// Newtype pattern around `uid_t` (which is just alias). It prevents bugs caused by accidentally /// passing wrong value. -#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Default)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub struct Uid(uid_t); impl Uid { @@ -79,7 +79,7 @@ pub const ROOT: Uid = Uid(0); /// /// Newtype pattern around `gid_t` (which is just alias). It prevents bugs caused by accidentally /// passing wrong value. -#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Default)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub struct Gid(gid_t); impl Gid { @@ -2423,7 +2423,7 @@ const PWGRP_BUFSIZE: usize = 1024; /// fields are based on the user's locale, which could be non-UTF8, while other fields are /// guaranteed to conform to [`NAME_REGEX`](https://serverfault.com/a/73101/407341), which only /// contains ASCII. -#[derive(Debug, Clone, PartialEq, Default)] +#[derive(Debug, Clone, PartialEq)] pub struct User { /// Username pub name: String, From 65ba0e5de95f3d1033f2ff25e27dc04c7b580218 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 11 Sep 2019 01:07:01 -0300 Subject: [PATCH 15/17] Use 16384 as limit Signed-off-by: Otavio Salvador --- src/unistd.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index e9b1933879..eb4d17e412 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -2482,9 +2482,10 @@ impl User { libc::size_t, *mut *mut libc::passwd) -> libc::c_int { + let buflimit = 16384; let bufsize = match sysconf(SysconfVar::GETPW_R_SIZE_MAX) { Ok(Some(n)) => n as usize, - Ok(None) | Err(_) => 1024 as usize, + Ok(None) | Err(_) => buflimit as usize, }; let mut cbuf = Vec::with_capacity(bufsize); @@ -2506,7 +2507,7 @@ impl User { } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic. - reserve_double_buffer_size(&mut cbuf, bufsize)?; + reserve_double_buffer_size(&mut cbuf, buflimit)?; } else { return Err(Error::Sys(Errno::last())); } @@ -2600,9 +2601,10 @@ impl Group { libc::size_t, *mut *mut libc::group) -> libc::c_int { + let buflimit = 16384; let bufsize = match sysconf(SysconfVar::GETGR_R_SIZE_MAX) { Ok(Some(n)) => n as usize, - Ok(None) | Err(_) => 1024 as usize, + Ok(None) | Err(_) => buflimit as usize, }; let mut cbuf = Vec::with_capacity(bufsize); @@ -2624,7 +2626,7 @@ impl Group { } } else if Errno::last() == Errno::ERANGE { // Trigger the internal buffer resizing logic. - reserve_double_buffer_size(&mut cbuf, bufsize)?; + reserve_double_buffer_size(&mut cbuf, buflimit)?; } else { return Err(Error::Sys(Errno::last())); } From 9111d1223e79f00b697d980578353ac62c0e20ac Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Wed, 11 Sep 2019 01:52:34 -0300 Subject: [PATCH 16/17] Rework more reservation code to use reserve_double_buffer_size Signed-off-by: Otavio Salvador --- src/unistd.rs | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index eb4d17e412..cd7fc0dcbe 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -1336,32 +1336,34 @@ pub fn setgid(gid: Gid) -> Result<()> { #[cfg(not(any(target_os = "ios", target_os = "macos")))] pub fn getgroups() -> Result> { // First get the number of groups so we can size our Vec - let ret = unsafe { libc::getgroups(0, ptr::null_mut()) }; + let ngroups = unsafe { libc::getgroups(0, ptr::null_mut()) }; + let ngroups_max = match sysconf(SysconfVar::NGROUPS_MAX) { + Ok(Some(n)) => n as usize, + Ok(None) | Err(_) => ::max_value(), + }; // Now actually get the groups. We try multiple times in case the number of // groups has changed since the first call to getgroups() and the buffer is // now too small. - let mut groups = Vec::::with_capacity(Errno::result(ret)? as usize); + let mut groups = Vec::::with_capacity(Errno::result(ngroups)? as usize); loop { // FIXME: On the platforms we currently support, the `Gid` struct has // the same representation in memory as a bare `gid_t`. This is not // necessarily the case on all Rust platforms, though. See RFC 1785. - let ret = unsafe { + let ngroups = unsafe { libc::getgroups(groups.capacity() as c_int, groups.as_mut_ptr() as *mut gid_t) }; - match Errno::result(ret) { + match Errno::result(ngroups) { Ok(s) => { unsafe { groups.set_len(s as usize) }; return Ok(groups); }, Err(Error::Sys(Errno::EINVAL)) => { - // EINVAL indicates that the buffer size was too small. Trigger - // the internal buffer resizing logic of `Vec` by requiring - // more space than the current capacity. - let cap = groups.capacity(); - unsafe { groups.set_len(cap) }; - groups.reserve(1); + // EINVAL indicates that the buffer size was too + // small. Trigger the internal buffer resizing logic + reserve_double_buffer_size(&mut groups, ngroups_max) + .or(Err(Error::Sys(Errno::EINVAL)))?; }, Err(e) => return Err(e) } @@ -1479,19 +1481,8 @@ pub fn getgrouplist(user: &CStr, group: Gid) -> Result> { // BSD systems will still fill the groups buffer with as many // groups as possible, but Linux manpages do not mention this // behavior. - - let cap = groups.capacity(); - if cap >= ngroups_max as usize { - // We already have the largest capacity we can, give up - return Err(Error::invalid_argument()); - } - - // Reserve space for at least ngroups - groups.reserve(ngroups as usize); - - // Even if the buffer gets resized to bigger than ngroups_max, - // don't ever ask for more than ngroups_max groups - ngroups = min(ngroups_max, groups.capacity() as c_int); + reserve_double_buffer_size(&mut groups, ngroups_max as usize) + .or(Err(Error::invalid_argument()))?; } } } From 3fe2c01c4aed2fbf0e9adf3a2a50d9e5b103e93f Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Sat, 21 Sep 2019 18:25:39 -0300 Subject: [PATCH 17/17] Rework getgroups() according to @asomers suggestions Signed-off-by: Otavio Salvador --- src/unistd.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/unistd.rs b/src/unistd.rs index cd7fc0dcbe..5de6305265 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -1337,15 +1337,18 @@ pub fn setgid(gid: Gid) -> Result<()> { pub fn getgroups() -> Result> { // First get the number of groups so we can size our Vec let ngroups = unsafe { libc::getgroups(0, ptr::null_mut()) }; + let mut groups = Vec::::with_capacity(Errno::result(ngroups)? as usize); + + // The value returned shall always be greater than or equal to one + // and less than or equal to the value of {NGROUPS_MAX} + 1. let ngroups_max = match sysconf(SysconfVar::NGROUPS_MAX) { - Ok(Some(n)) => n as usize, + Ok(Some(n)) => (n + 1) as usize, Ok(None) | Err(_) => ::max_value(), }; - // Now actually get the groups. We try multiple times in case the number of - // groups has changed since the first call to getgroups() and the buffer is - // now too small. - let mut groups = Vec::::with_capacity(Errno::result(ngroups)? as usize); + // We try multiple times in case the number of groups has changed + // since the first call to getgroups() and the buffer is now too + // small. loop { // FIXME: On the platforms we currently support, the `Gid` struct has // the same representation in memory as a bare `gid_t`. This is not