Skip to content

Conversation

@Superhepper
Copy link
Collaborator

@Superhepper Superhepper commented Jan 31, 2023

Handles returned from tpm2-tss APIs does not need to be unique. This an attempt to fix problem #383.

This problem was also discussed in tpm2-software/tpm2-tss#2537.

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

@ionut-arm
Copy link
Member

Will need to port this to v7 as well. I can open another PR after I merge the current one.

@Superhepper Superhepper marked this pull request as ready for review January 31, 2023 22:07
@Superhepper Superhepper force-pushed the non-unique-handles branch 3 times, most recently from f7b5a6c to 8cf029d Compare February 1, 2023 19:42
@aplanas
Copy link

aplanas commented Feb 2, 2023

This should be already merged in #390? Safe to close?

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.

👍🏻

@ionut-arm
Copy link
Member

This should be already merged in #390? Safe to close?

Depends on whether @Superhepper prefers getting rid of the unwrap or leaving as is - I, for one, don't mind either way.

@Superhepper
Copy link
Collaborator Author

This basically the same thing this is just to master.

@Superhepper Superhepper requested a review from wiktor-k February 2, 2023 18:24
error!("Handle({}) is already open", ESYS_TR::from(handle));
return Err(Error::local_error(WrapperErrorKind::InvalidHandleState));
// It is safe to call unwrap because the existance of the key has already been verified.
let stored_handle_drop_action = self.open_handles.get(&handle).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's possible to avoid the unwrap by using if let just like in the other PR.

I trust you'll adjust it and I don't want to unnecessarily block the workflow so I'll ack the PR now :)

- Handles returned from tpm2-tss APIs does not need to be unique.
  This an attempt to fix problem parallaxsecond#383 by removing the check in
  HandleManager that made a check for uniqueness. This has been replaced
  with handle consistency check.

- Enables the integration tests that was disabled due to this issue.

Co-authored-by: Ionut Mihalcea <ionut.mihalcea@arm.com>
Co-authored-by: Jesper Brynolf <jesper.brynolf@gmail.com>

Signed-off-by: Jesper Brynolf <jesper.brynolf@gmail.com>
@Superhepper Superhepper merged commit bea611a into parallaxsecond:main Feb 2, 2023
@Superhepper Superhepper deleted the non-unique-handles branch December 16, 2023 08:58
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