Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add getpwnam and related functions #864

Closed
wants to merge 17 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 28 additions & 18 deletions src/unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -539,6 +539,23 @@ pub fn symlinkat<P1: ?Sized + NixPath, P2: ?Sized + NixPath>(
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<T>(buf: &mut Vec<T>, limit: usize) -> Result<()> {
use std::cmp::min;

if buf.len() == limit {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be >=, not ==

return Err(Error::Sys(Errno::ERANGE))
}

unsafe { buf.set_len(buf.capacity()) };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, absolutely not. This is very unsafe and totally unnecessary. Did you mean to commit this line?


let capacity = min(buf.capacity() * 2, limit);
buf.reserve_exact(capacity);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to override the standard library. You can use reserve instead of reserve_exact. I just don't want to rely on reserve(1) actually reserving more space than 1.


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
Expand Down Expand Up @@ -581,11 +598,8 @@ pub fn getcwd() -> Result<PathBuf> {
}
}

// 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)?;
}
}
}
Expand Down Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that it's defined in any publicly visible place, but FreeBSD internally sets this limit to 1MB.

};

let mut cbuf = Vec::with_capacity(bufsize);
let mut cbuf = Vec::with_capacity(512);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were right the first time. GETPW_R_SIZE_MAX is the suggested initial size of the buffer for password entries, not the maximum size (earlier versions of POSIX did specify the maximum size).

let mut pwd = mem::MaybeUninit::<libc::passwd>::uninit();
let mut res = ptr::null_mut();

Expand All @@ -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()));
}
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, bufsize should be the intial sizes of the vec, not the maximum.

let mut grp = mem::MaybeUninit::<libc::group>::uninit();
let mut res = ptr::null_mut();

Expand All @@ -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()));
}
Expand Down