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 setresuid and setresgid #448

Merged
merged 1 commit into from
Oct 27, 2016
Merged

Add setresuid and setresgid #448

merged 1 commit into from
Oct 27, 2016

Conversation

dgreid
Copy link

@dgreid dgreid commented Oct 25, 2016

These were both recently added to libc, add wrappers.

Signed-off-by: Dylan Reid dgreid@chromium.org

@posborne
Copy link
Member

Looks good. A couple of minor things:

  1. Go ahead and add an entry to the CHANGELOG.md with a note on the additions
  2. Although most functions are not documented at present, I think we are moving in that direction. Go ahead and add documentation for the functions which includes a summary description and link to the relevant online man pages (on man7.org)

Copy link
Contributor

@fiveop fiveop left a comment

Choose a reason for hiding this comment

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

Just a small request regarding the markup in CHANGELOG.md

@@ -12,6 +12,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- Added function `epoll_create1` and bitflags `EpollCreateFlags` in
`::nix::sys::epoll` in order to support `::libc::epoll_create1`.
([#410](https://github.com/nix-rust/nix/pull/410))
- Added setresuid and setresgid for Linux
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please markup the identifiers as code and reference the module they live in (::nix::unistd).

@@ -587,6 +588,38 @@ mod linux {
Errno::result(res).map(drop)
}

/// Sets the real, effective, and saved uid
Copy link
Member

Choose a reason for hiding this comment

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

This is a real nit, but when rendered I think this will juxtapose the first line and "Further Reading", so it probably makes sense to add a period after the first line or do something like ([see setresuid(2)][...])

@posborne
Copy link
Member

Thanks for making the updates!

These were both recently added to libc, add wrappers.

Signed-off-by: Dylan Reid <dgreid@chromium.org>

---
Changes since v2:
Updated function comments and CHANGELOG

Changes since v1:
Add function comments, update CHANGELOG
@posborne
Copy link
Member

Looks good. @homu r+

@homu
Copy link
Contributor

homu commented Oct 27, 2016

📌 Commit 6c85f43 has been approved by posborne

@homu
Copy link
Contributor

homu commented Oct 27, 2016

⚡ Test exempted - status

@homu homu merged commit 6c85f43 into nix-rust:master Oct 27, 2016
homu added a commit that referenced this pull request Oct 27, 2016
Add setresuid and setresgid

These were both recently added to libc, add wrappers.

Signed-off-by: Dylan Reid <dgreid@chromium.org>
@fiveop
Copy link
Contributor

fiveop commented Oct 28, 2016

Thanks

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

4 participants