-
Notifications
You must be signed in to change notification settings - Fork 61
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
Added new methods to fix issue 375 - get slots with initialized token… #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Made a few comments 👌
// Copyright 2021 Contributors to the Parsec project. | ||
// SPDX-License-Identifier: Apache-2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's ok to keep those lines :)
let mut slot_count = 0; | ||
|
||
unsafe { | ||
Rv::from(get_pkcs11!(self, C_GetSlotList)( | ||
cryptoki_sys::CK_TRUE, | ||
std::ptr::null_mut(), | ||
&mut slot_count, | ||
)) | ||
.into_result()?; | ||
} | ||
|
||
let mut slots = vec![0; slot_count.try_into()?]; | ||
|
||
unsafe { | ||
Rv::from(get_pkcs11!(self, C_GetSlotList)( | ||
cryptoki_sys::CK_TRUE, | ||
slots.as_mut_ptr(), | ||
&mut slot_count, | ||
)) | ||
.into_result()?; | ||
} | ||
|
||
let mut slots: Vec<Slot> = slots.into_iter().map(Slot::new).collect(); | ||
|
||
// This should always truncate slots. | ||
slots.resize(slot_count.try_into()?, Slot::new(0)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can replace all those lines with:
let slots = self.get_slots_with_token()?;
It should do the same using the method above!
You might need to do a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!!
} | ||
Err(e) => Some(Err(e)), | ||
}) | ||
.collect::<Result<Vec<Slot>>>()?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is minor but you can either omit type declaration in let slots: Vec<Slot>
or shorten the explicit type in collect to collect::<Result<_>>()?;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this function returns a `Result<Vec>, I am wondering if you could also remove everything:
- the
let slots: Vec<Slot> =
at the beginning - just have
.collect()
at the end (without?
and;
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. That'd make it more readable too IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
…, for PKCS 11 Provider Signed-off-by: Adriano Pallavicino <[email protected]>
…, for PKCS 11 Provider
Signed-off-by: Adriano Pallavicino [email protected]