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 support for request_key and keyctl_instantiate #13

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mythi
Copy link

@mythi mythi commented Aug 20, 2024

Fixes: #12

@landhb landhb self-requested a review August 21, 2024 21:35
@landhb
Copy link
Owner

landhb commented Aug 22, 2024

@mythi Thanks for the PR!

It looks great, will try to finish the review this weekend. Left one comment for your question about "NULL" callouts.

@mythi
Copy link
Author

mythi commented Aug 23, 2024

It looks great, will try to finish the review this weekend. Left one comment for your question about "NULL" callouts.

Thanks, I think the comment is missing (at least I cannot see it).

I pushed one small fix to examples/keyctl (changed keyring to ring because of a conflicting -k shorname for key/keyring)

src/keyring.rs Outdated
pub fn request_key<D: AsRef<str> + ?Sized, C: AsRef<str> + ?Sized>(
&self,
description: &D,
callout: &C,
Copy link
Owner

Choose a reason for hiding this comment

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

I think having this as Option<&C> and then updating ffi::request_key to provide core::ptr::null when the Option is None would be nice from a user perspective. Instead of checking for ""

Copy link
Author

Choose a reason for hiding this comment

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

I thought about the same but dropped the idea in the initial ver. I've have that now implemented. AFAUI, Option<&C> wants type hint so the param becomes None::<&str>.

@landhb landhb self-requested a review August 23, 2024 17:05
@mythi mythi force-pushed the request-key branch 2 times, most recently from 70758d7 to d76f9f2 Compare August 26, 2024 10:18
@mythi
Copy link
Author

mythi commented Sep 8, 2024

(pushed with one lint error fixed that I just discovered)

mythi added 6 commits October 30, 2024 20:36
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
the rustdoc content is based on 'man request_key'.

Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
Signed-off-by: Mikko Ylinen <mikko.ylinen@intel.com>
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.

keyctl instantiate
2 participants