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

Implement tr_serialize and tr_deserialize #461

Merged
merged 1 commit into from
Nov 6, 2023

Conversation

brandsimon
Copy link
Contributor

@brandsimon brandsimon commented Oct 25, 2023

I don't know if the implementation with buffer.len().try_into().map_err and std::slice::from_raw_parts is the cleanest solution. Please let me know if you have ideas for cleaner solutions.
Regarding the tests, the new_handle = old_handle + 1, but I think checking the public part (which includes the public key) is the most relevant test for it.

Edit:
The test with the new context is needed. Currently it works with the old context, but not with a new one.

@brandsimon brandsimon force-pushed the sbr/serialize branch 8 times, most recently from b8eb40e to 3905696 Compare October 25, 2023 15:52
@brandsimon brandsimon changed the title Implement tr_serialize and tr_deserialize WIP: Implement tr_serialize and tr_deserialize Oct 25, 2023
@brandsimon brandsimon force-pushed the sbr/serialize branch 5 times, most recently from 9c98521 to 5dc42e9 Compare October 25, 2023 16:37
@brandsimon
Copy link
Contributor Author

brandsimon commented Oct 26, 2023

@Superhepper
Thank you very much. Your ffi code works great and looks clean.
I changed the test to persist the handle and drop the old context. If the persistent handle exists, it is cleared before the test.

@brandsimon brandsimon changed the title WIP: Implement tr_serialize and tr_deserialize Implement tr_serialize and tr_deserialize Oct 26, 2023
Copy link
Member

@ionut-arm ionut-arm 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 for the patch!

tss-esapi/src/context/general_esys_tr.rs Outdated Show resolved Hide resolved
@brandsimon brandsimon force-pushed the sbr/serialize branch 2 times, most recently from a3af865 to 3122e78 Compare October 28, 2023 23:59
@brandsimon brandsimon force-pushed the sbr/serialize branch 2 times, most recently from cacf52e to ecb9624 Compare November 1, 2023 19:25
@brandsimon
Copy link
Contributor Author

@ionut-arm @Superhepper
I added descriptions and examples.

Superhepper
Superhepper previously approved these changes Nov 1, 2023
Copy link
Collaborator

@Superhepper Superhepper left a comment

Choose a reason for hiding this comment

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

LGTM!

Thank you for your hard work.

@Superhepper Superhepper requested a review from wiktor-k November 1, 2023 20:57
wiktor-k
wiktor-k previously approved these changes Nov 1, 2023
Copy link
Collaborator

@wiktor-k wiktor-k left a comment

Choose a reason for hiding this comment

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

I think it looks good 👍 I found only very minor things.

tss-esapi/src/context/general_esys_tr.rs Outdated Show resolved Hide resolved
Signed-off-by: Simon Brand <simon.brand@postadigitale.de>
Copy link
Member

@ionut-arm ionut-arm 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 so much for the effort!

@ionut-arm ionut-arm merged commit 21d4263 into parallaxsecond:main Nov 6, 2023
10 checks passed
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.

4 participants