libpriv/passwd: move passwd database to Rust#2348
Conversation
| { | ||
| GLNX_AUTO_PREFIX_ERROR ("Converting /var to tmpfiles.d", error); | ||
|
|
||
| g_autoptr(RpmOstreePasswdDB) pwdb = rpmostree_passwddb_open (rootfs_dfd, cancellable, error); |
There was a problem hiding this comment.
Note: the cancellable was not really used even in the C logic, so I just dropped it here.
|
Self-shaming: I originally made a typo-mistake when porting the logic, and CI saved my bacon. |
|
I'm not opposed to this and am OK merging mostly as is if you prefer but the added ratio of "unsafe FFI glue" here is the same reason I abandoned my attempt to use this approach for oxidizing the origin file. To me the biggest value using Rust comes when we have nontrivial logic inside the Rust. The original usage of parsing the treefile was a perfect example; serde gives so much better error messages, allowed us to implement treefile So WDYT about trying this again after #2336 lands? It's getting closer. |
|
/hold |
|
Rebase needed |
|
OK (next year or whenever you're ready) want to take a crack at rebasing on cxx-rs work from #2336 ? |
|
/hold cancel Reworked on top of |
| let c_path: CUtf8Buf = passwd_path.to_string().into(); | ||
| let db_ptr = self as *mut Self; | ||
| let mut gerror: *mut glib_sys::GError = std::ptr::null_mut(); | ||
| // TODO(lucab): find a replacement for `fgetpwent` and drop this. |
There was a problem hiding this comment.
I'd like to get rid of the TODO after landing this PR. I can either look at adding fgetpwent_r to the libc crate, or port the existing logic to Rust.
I have a slight preference for the latter as it is a small line-based parser, and the unsafe string handling is quite cumbersome anyway. @cgwalters @jlebon thoughts?
There was a problem hiding this comment.
Yeah, rewriting in Rust seems probably better here - if there's an existing crate that'd be an even more obvious path. But it's a simple format anyways.
There was a problem hiding this comment.
https://github.com/kstep/parsswd looks a bit stale but it's the best candidate I found on crates.io.
But it doesn't look much aligned with the glibc logic above. I'll spelunk a bit more, possibly upstreaming a port of the glibc parser there.
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Overall LGTM, a few minor bits.
| match self.groups.get(&key) { | ||
| Some(group) => Ok(group.clone()), | ||
| None => bail!("failed to find group ID '{}'", gid), | ||
| } |
There was a problem hiding this comment.
This is totally fine, I am just personally trying to use combinators more lately, so it'd look more like
self.groups.get(&key).cloned().ok_or_else(|| anyhow!("failed to find group ID '{}'", gid))
| let c_path: CUtf8Buf = passwd_path.to_string().into(); | ||
| let db_ptr = self as *mut Self; | ||
| let mut gerror: *mut glib_sys::GError = std::ptr::null_mut(); | ||
| // TODO(lucab): find a replacement for `fgetpwent` and drop this. |
There was a problem hiding this comment.
Yeah, rewriting in Rust seems probably better here - if there's an existing crate that'd be an even more obvious path. But it's a simple format anyways.
| if res != 0 { | ||
| Ok(()) | ||
| } else { | ||
| anyhow::ensure!(!gerror.is_null(), "unknown failure (NULL Gerror)"); |
There was a problem hiding this comment.
I'd lean towards aborting here (i.e. panic!()) since it's a bad bug if it happens, and it really shouldn't - it's definitely not a "user error".
There was a problem hiding this comment.
Ack, I've turned this one into an assert!(),
This moves to Rust the in-memory structure holding passwd entries (users and groups).
|
Thanks a ton for holding on this until the cxx-rs stuff had landed! I think the result is worth it. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, lucab The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
6 participants
This moves to Rust the in-memory structure holding passwd entries
(users and groups).