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

WinSCard FFI implementation #205

Merged
merged 55 commits into from
Feb 5, 2024

Conversation

TheBestTvarynka
Copy link
Collaborator

@TheBestTvarynka TheBestTvarynka commented Feb 2, 2024

Hi,
This pull request contains the WinSCard FFI implementation.

The WinSCard FFI is very tricky because Microsoft designed this API with complications. For example, during the smart card context deletion, we also need to free all allocated resources by this context (like smart card handles and buffers). For every dangerous/critical place, I added a comment about it.

I can't make this PR smaller 😞 . The smart card context and smart card handles are very coupled together, so I can't split this PR into two.

Docs & references:

@TheBestTvarynka TheBestTvarynka self-assigned this Feb 2, 2024
@TheBestTvarynka TheBestTvarynka marked this pull request as ready for review February 2, 2024 19:22
Copy link
Member

@CBenoit CBenoit left a comment

Choose a reason for hiding this comment

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

Thank you! Well documented again. 👍
I left a few comments that you can address and merge yourself. (Since we are merging into a dev branch, I think you should have the permissions.)

note: The commit type as per conventional commit specification for refactoring is refactor instead refac. If you want to stick to the convention.

crates/ffi-types/src/winscard/functions.rs Show resolved Hide resolved
ffi/Cargo.toml Outdated Show resolved Hide resolved
ffi/src/winscard/scard_handle.rs Outdated Show resolved Hide resolved
ffi/src/winscard/scard.rs Outdated Show resolved Hide resolved
ffi/src/winscard/buff_alloc.rs Outdated Show resolved Hide resolved
@TheBestTvarynka
Copy link
Collaborator Author

I answered the comments but didn't resolve them and left them to you. Maybe you will have any other questions

@TheBestTvarynka TheBestTvarynka merged commit 59195a7 into winscard-rs-impl Feb 5, 2024
40 checks passed
@TheBestTvarynka TheBestTvarynka deleted the feature/winscard-ffi-impl branch February 5, 2024 10:50
@CBenoit
Copy link
Member

CBenoit commented Feb 5, 2024

I answered the comments but didn't resolve them and left them to you. Maybe you will have any other questions

Thank you! Everything is good! 🙆‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants