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

Expose getpwuid #1140

Closed
wants to merge 1 commit into from
Closed

Expose getpwuid #1140

wants to merge 1 commit into from

Conversation

FauxFaux
Copy link

Implement getpwuid_r, which loads the passwd entry for a given Uid, allowing you to find the user's name, shell, homedir, etc.

I did not know what to do about the string ownership model. It returns us a struct full of *const c_char pointers, which supposedly point to our buffer. I wrote new code to check these, to some extent, then trust CStr to do the rest. I did this because I couldn't find any other code in nix that did this (which must exist, someone point it out so I can slap my forehead), and to defer the cost of the checks to the time of use, i.e. so if you only want the name, you don't have to pay to check the shell.

The code should be safe without the extra checks, if the system follows the rules, and I have no reason to believe otherwise; perhaps just deleting the extra checks is reasonable?

Also not handled: dynamically increasing the buffer size if the function returns that the buffer is too small. GETPW_R_SIZE_MAX is 1024 on Ubuntu 19.10 / 5.3 / amd64.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

I wonder if the range checking should be moved into getpwduid itself, instead of being done on every access. What do you think?

///
/// See also [getpwuid(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid.html)
pub fn getpwuid(uid: Uid) -> Result<Option<Passwd>> {
let mut inner: libc::passwd = unsafe { mem::zeroed() };
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 use mem::MaybeUninit instead of mem::zeroed.

/// See also [getpwuid(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/getpwuid.html)
pub fn getpwuid(uid: Uid) -> Result<Option<Passwd>> {
let mut inner: libc::passwd = unsafe { mem::zeroed() };
let mut result = ptr::null_mut();
Copy link
Member

Choose a reason for hiding this comment

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

result, too, should use MaybeUninit.

@@ -473,6 +473,24 @@ cfg_if!{
}
}

#[test]
fn test_passwd() -> nix::Result<()> {
let passwd = getpwuid(getuid())?
Copy link
Member

Choose a reason for hiding this comment

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

Test functions should all return ().

@asomers
Copy link
Member

asomers commented Oct 20, 2019

You should probably look at #1139. It overlaps with this one.

@otavio
Copy link
Contributor

otavio commented Oct 21, 2019

@FauxFaux please look at #1139 and see if you could provide any feedback on this.

@asomers
Copy link
Member

asomers commented Oct 30, 2019

I'm going to close this PR, because I like #1139's approach better. It solves the same problem but in a different way.

@asomers asomers closed this Oct 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants