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 newtypes for UserId, GroupId and ProcessId #769

Closed
rnijveld opened this issue Sep 19, 2023 · 3 comments · Fixed by #879
Closed

Add newtypes for UserId, GroupId and ProcessId #769

rnijveld opened this issue Sep 19, 2023 · 3 comments · Fixed by #879
Assignees
Labels
minor minor issue, PR without an issue

Comments

@rnijveld
Copy link
Collaborator

rnijveld commented Sep 19, 2023

We currently use type aliases, but that doesn't really add any type safety. Group ids, process ids and user ids can freely be interchanged because their definitions in libc are the same. If we use newtype wrappers (i.e. struct UserId(libc::uid_t) instead of type UserId = libc::uid_t), we can actually make a few more ensurances that they come from the right source. We should make construction of these newtypes very much explicit because of that (i.e. no From). That should prevent accidental conversion into one of the newtype definitions where it was not meant.

@pvdrz
Copy link
Collaborator

pvdrz commented Sep 20, 2023

I'm stealing this :trollface:

@pvdrz pvdrz self-assigned this Sep 20, 2023
@squell squell unassigned pvdrz Jan 5, 2024
@squell squell added minor minor issue, PR without an issue good first issue Good for newcomers labels Aug 26, 2024
@van-sprundel
Copy link
Contributor

I would like to work on this

UserId is used as const in timestamp tests, would it be okay to add lazy_static as a test dependency? Or add a const new method behind a test flag?

@squell squell linked a pull request Oct 14, 2024 that will close this issue
@squell
Copy link
Member

squell commented Oct 14, 2024

About lazy_static; can't we achieve the same thing with the now-standard "once cell" features? rust-lang-nursery/lazy-static.rs#214 (I recall once having used it to provide backwards compatibility to rustc 1.65 during early sudo-rs development).

Our MSRV is currently 1.70 (and I think we can easily be persuaded to up it as far as 1.75), so once_cell is available.

@squell squell removed the good first issue Good for newcomers label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor minor issue, PR without an issue
Projects
None yet
4 participants