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.

Sign up for GitHub

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

Jump to bottom

Refactoring Phase 1 increment 2. #96

Merged
merged 1 commit into from
Jul 17, 2020
Merged

Refactoring Phase 1 increment 2. #96

merged 1 commit into from
Jul 17, 2020

Conversation

Superhepper
Copy link
Collaborator

@Superhepper Superhepper commented Jul 14, 2020 •
edited
Loading

Refactoring phase one increment 2. Is part of the #87 issue.

-- Renamed PcrSelections to PcrSelectionList and moved it to
structures/lists
-- Moved DigestList to structures/lists
-- Added dir structures/pcr
-- Created PcrSelection and PcrSelect.
-- Added Copyright info to the new files.

Signed-off-by: Jesper Brynolf jesper.brynolf@gmail.com

@Superhepper Superhepper marked this pull request as ready for review July 15, 2020 13:36
@hug-dev
Copy link
Member

hug-dev commented Jul 15, 2020

Thanks a lot for doing the re-organisation into something that makes sense and respects the TSS structure 👌

For the lists: @ionut-arm suggested in an older PR that we could also have our own list wrapper type over a generic parameter type. Are TSS lists defined somewhere? If it can always be represented as a Vec of something, then maybe we can defined our List<T> type and only having to define the T abstractions and the conversions between our List<T> and the TPML_... types? That would avoid code duplication if it can work in all cases!

About copyrights for new files: it might be better to put 2020 as the year.

@Superhepper
Copy link
Collaborator Author

Thanks a lot for doing the re-organisation into something that makes sense and respects the TSS structure 👌

For the lists: @ionut-arm suggested in an older PR that we could also have our own list wrapper type over a generic parameter type. Are TSS lists defined somewhere? If it can always be represented as a Vec of something, then maybe we can defined our List<T> type and only having to define the T abstractions and the conversions between our List<T> and the TPML_... types? That would avoid code duplication if it can work in all cases!

About copyrights for new files: it might be better to put 2020 as the year.

Are TSS lists defined some where? Yes, in the specification(Part 2: Structures, Chapter 10.9 Lists).

To have our own List type would be nice though I am not sure right now if it is possible but absolutely something that should be investigated. Pcr Selection List as an example would need to be rewritten in that case. Because it uses a HashMap in order to find selections associated with the same HashingAlgorithm and squash them together.

@hug-dev
Copy link
Member

hug-dev commented Jul 15, 2020

Yes sure, I don't want to put that as a work to do for this PR 😃 Let's create an issue though!

@Superhepper Superhepper force-pushed the refactoring_phase_1v2 branch 2 times, most recently from 84c5b96 to bb86f0c Compare July 16, 2020 07:41
@Superhepper
Copy link
Collaborator Author

Fixed some of the copyright and added a test for DigestList.

ionut-arm
ionut-arm reviewed Jul 16, 2020
View reviewed changes
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.

Thanks! Looking really good, I left a few minor comments below

src/algorithm/mod.rs Outdated Show resolved Hide resolved
src/algorithm/structures/mod.rs Outdated Show resolved Hide resolved
src/structures/lists/pcr_selection.rs Outdated Show resolved Hide resolved
@hug-dev
Copy link
Member

hug-dev commented Jul 17, 2020

Hi @Superhepper ,

If you rebase your PR, then we can merge it!

@hug-dev hug-dev mentioned this pull request Jul 17, 2020
Merged
@Superhepper Superhepper force-pushed the refactoring_phase_1v2 branch 2 times, most recently from cd4ae60 to 642ae31 Compare July 17, 2020 14:15
@Superhepper
Refactoring Phase 1 increment 2.
969c8ea 
-- Renamed PcrSelections to PcrSelectionList and moved it to
   structures/lists
-- Moved DigestList to structures/lists
-- Added dir structures/pcr
-- Created PcrSelection and PcrSelect.
-- Added Copyright info to the new files.

Signed-off-by: Jesper Brynolf <jesper.brynolf@gmail.com>
@Superhepper
Copy link
Collaborator Author

Done

ionut-arm
ionut-arm approved these changes Jul 17, 2020
View reviewed changes
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.

LGTM!

@ionut-arm ionut-arm merged commit b9e1b19 into parallaxsecond:master Jul 17, 2020
@Superhepper Superhepper deleted the refactoring_phase_1v2 branch October 2, 2020 12:34
tgonzalezorlandoarm pushed a commit to tgonzalezorlandoarm/rust-tss-esapi that referenced this pull request Mar 14, 2024
@ionut-arm
Merge pull request parallaxsecond#96 from ionut-arm/len-limit
e3ce785 
Add body length limit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ionut-arm ionut-arm ionut-arm approved these changes

@hug-dev hug-dev hug-dev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Superhepper @hug-dev @ionut-arm