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

Add EFI_TIMESTAMP_PROTOCOL #37

Closed
wants to merge 1 commit into from
Closed

Conversation

Ayush1325
Copy link
Contributor

I am thinking of using it to implement std::time::Instant.

Signed-off-by: Ayush Singh ayushsingh1325@gmail.com

Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>
Copy link
Member

@dvdhrm dvdhrm left a comment

Choose a reason for hiding this comment

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

Minor comments inline. I fixed them up myself. If I screwed things up, or if you disagree, please let me know and we can discuss it in detail for the next release.

Thanks a lot!


#[repr(C)]
pub struct Properties {
frequence: u64,
Copy link
Member

Choose a reason for hiding this comment

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

frequence -> frequency

);

#[repr(C)]
pub struct Properties {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should derive Clone, Copy, and Debug here.

#[repr(C)]
pub struct Protocol {
get_timestamp: GetTimestamp,
get_properties: GetProperties,
Copy link
Member

Choose a reason for hiding this comment

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

These members should be marked pub, and I prefixed the types with Protocol* for namespacing.

@dvdhrm dvdhrm closed this Jul 6, 2022
@Ayush1325
Copy link
Contributor Author

Thanks for fixing those mistakes.

@Ayush1325 Ayush1325 deleted the timestamp branch July 6, 2022 11:12
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.

None yet

2 participants