Teeerex: Be much more verbose during the process. #181
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OverOrion
force-pushed
the
szp/attesteer-debug
branch
from
40cd6b3
to
8e6b8dc
Compare
OverOrion
force-pushed
the
szp/attesteer-debug
branch
from
64464da
to
fd56641
Compare
OverOrion
changed the title
OverOrion
force-pushed
the
szp/attesteer-debug
branch
from
fd56641
to
e9cd2ce
Compare
clangenb
reviewed
Jul 4, 2023
primitives/teerex/src/lib.rs
Outdated
| @@ -143,10 +143,16 @@ impl TcbVersionStatus { | |||
|
|
|||
| pub fn verify_examinee(&self, examinee: &TcbVersionStatus) -> bool { | |||
| for (v, r) in self.cpusvn.iter().zip(examinee.cpusvn.iter()) { | |||
| log::debug!("teerex: verify_examinee: v={:#?},r={:#?}", v, r); | |||
There was a problem hiding this comment.
Thanks a lot for all these extra logs. However, I am not fond of teerex:, it is essentially redundant information as the logger does already contain the rust module path. But we could add a target: TEEREX as an argument, where teerex should be a const ;)
There was a problem hiding this comment.
I did not know about this, handy feature! Rewrote all of them to use it, thanks for the early review! :D 💯
teerex/sgx-verify/src/collateral.rs
Outdated
Comment on lines
233
to
235
| log::info!("teerex: called into runtime call register_tcb_info(), inside Self::is_valid."); | ||
| log::info!("teerex: called into runtime call register_tcb_info(), inside Self::is_valid, self is: {:#?}" , &self); | ||
| log::info!("teerex: called into runtime call register_tcb_info(), inside Self::is_valid, timestamp_millis: {:#?}" , ×tamp_millis); |
There was a problem hiding this comment.
Together with the above suggestion of setting a target instead of just having the teerex in the string I suggest:
Suggested change
| log::info!("teerex: called into runtime call register_tcb_info(), inside Self::is_valid."); | |
| log::info!("teerex: called into runtime call register_tcb_info(), inside Self::is_valid, self is: {:#?}" , &self); | |
| log::info!("teerex: called into runtime call register_tcb_info(), inside Self::is_valid, timestamp_millis: {:#?}" , ×tamp_millis); | |
| log::info!(target: TEEREX, "Called into runtime call register_tcb_info(), inside Self::is_valid."); | |
| log::info!(target: TEEREX, "self is: {:#?}" , &self); | |
| log::info!(target: TEEREX, "timestamp_millis: {:#?}" , ×tamp_millis); |
What do you think?
OverOrion
force-pushed
the
szp/attesteer-debug
branch
from
e9cd2ce
to
f384c41
Compare
clangenb
reviewed
Jul 4, 2023
clangenb
approved these changes
Jul 4, 2023
|
Sorry, for merging the other one first. I remember that Alain brought in a `log::info!("teerex: ***"), maybe you can fix this while resolving the conflicts. :) |
brenzi
approved these changes
Jul 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.