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 get_token_info #27

Merged
merged 1 commit into from
Jun 18, 2021
Merged

Conversation

wiktor-k
Copy link
Collaborator

Okay folks, I'll need some more help on this one.

  • I'm not sure where to place the TokenInfo struct (should it be in the -sys crate?)
  • about the struct members themselves: for example currently they are raw types that are read from the PKCS#11 driver but maybe it would be nice to clean them up (e.g. all strings - as usual in PKCS#11 - are space padded - see the unit test).
  • get_token_info uses some raw ffi code I found that works but I'm not sure if that's the best approach.

Please ignore placeholder docs for a moment. I'll add them when other issues are fixed :)

@wiktor-k wiktor-k force-pushed the add-get-token-info branch from 6cb818f to 2679ef7 Compare June 17, 2021 11:44
@hug-dev hug-dev added the enhancement New feature or request label Jun 17, 2021
cryptoki/src/types/slot_token.rs Outdated Show resolved Hide resolved
cryptoki/src/types/slot_token.rs Outdated Show resolved Hide resolved
@ionut-arm
Copy link
Member

I'm not sure where to place the TokenInfo struct (should it be in the -sys crate?)

I think the place where you put them is the right one. That file/module is supposed to cover section 3.2 Slot and token types of the spec

This function can be used to retrieve manufacturer ID or serial number
of the token in the slot.

Signed-off-by: Wiktor Kwapisiewicz <wiktor@metacode.biz>
@wiktor-k wiktor-k force-pushed the add-get-token-info branch from 2679ef7 to d135273 Compare June 18, 2021 06:39
@wiktor-k
Copy link
Collaborator Author

Okay, I've vastly simplified the code when it occurred to me that CK_TOKEN_INFO generated by bindgen has already all members that I needed (btw is it possible somehow to see the code generated by bindgen? IDE doesn't see it just goes to macros :( ).

I've converted TokenInfo to be a wrapper around CK_TOKEN_INFO with accessor methods for properties that I use.

I've also added Deref and From implementations to be somehow similar to what you use with other types. (This can be used to access properties that don't have convenience accessor functions).

@wiktor-k wiktor-k changed the title WIP: Add get_token_info Add get_token_info Jun 18, 2021
@wiktor-k wiktor-k marked this pull request as ready for review June 18, 2021 06:47
@ionut-arm
Copy link
Member

ionut-arm commented Jun 18, 2021

btw is it possible somehow to see the code generated by bindgen? IDE doesn't see it just goes to macros :(

We already have bindings commited for a number of architectures, you can see them here - this is to have faster/easier builds on those platforms. Otherwise, you should be able to find them in something like ./target/debug/build/cryptoki-sys-.../out/cryptoki-sys-bindings.rs, if I remember correctly. Is this the code you meant?

@wiktor-k
Copy link
Collaborator Author

Is this the code you meant?

Yes! The builds for architectures are great for looking for what's available, thanks @ionut-arm! I didn't spot this previously.

Copy link
Member

@hug-dev hug-dev left a comment

Choose a reason for hiding this comment

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

Looks good 👌

@wiktor-k wiktor-k requested a review from ionut-arm June 18, 2021 11:56
@ionut-arm ionut-arm merged commit aa13908 into parallaxsecond:main Jun 18, 2021
@wiktor-k wiktor-k deleted the add-get-token-info branch June 18, 2021 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants