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_SHELL_PROTOCOL #51

Closed
wants to merge 1 commit into from
Closed

Conversation

Ayush1325
Copy link
Contributor

Defined in UEFI Shell Specification.

I have left all the pointers as *mut for now. But I think it might be possible to change some of them to *const.

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

Defined in UEFI Shell Specification

Signed-off-by: Ayush <ayushsingh1325@gmail.com>
@Ayush1325
Copy link
Contributor Author

Ayush1325 commented Aug 30, 2022

The failed CI is probably due to rust-lang/rust#101071

@Ayush1325
Copy link
Contributor Author

Hi @dvdhrm , can you rerun the CI now that rust-lang/rust#101071 has been merged? I think it should be fixed now.

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 fixes that I have queued locally. Just one question before I merge it regarding FileHandle. See inline. I can do the fixup myself.

) -> *mut crate::protocols::device_path::Protocol};

pub type GetFilePathFromDevicePath = eficall! {fn(
*mut *mut crate::protocols::device_path::Protocol,
Copy link
Member

Choose a reason for hiding this comment

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

This should be *mut rather than *mut *mut, right?

Copy link
Contributor Author

@Ayush1325 Ayush1325 Sep 5, 2022

Choose a reason for hiding this comment

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

This should be *mut rather than *mut *mut, right?

Yes

Copy link
Member

Choose a reason for hiding this comment

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

No, I already have it queued locally.

&[0x60, 0xc9, 0xfe, 0xf5, 0xda, 0x4e],
);

pub type ShellFileHandle = *mut core::ffi::c_void;
Copy link
Member

Choose a reason for hiding this comment

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

This is actually SHELL_FILE_HANDLE in the spec? Why does this not have the EFI_* prefix? sigh

If it was EFI_SHELL_FILE_HANDLE, we would call it FileHandle (drop the shell prefix). What do you think, should we rename to FileHandle to be more consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it would be fine to rename it to FileHandle since it is not used in any other Shell Protocols.

}

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

Choose a reason for hiding this comment

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

This is called EFI_SHELL_FILE_INFO in the spec, so we should really name it FileInfo.

@dvdhrm
Copy link
Member

dvdhrm commented Sep 5, 2022

Perfect! Merged. Thanks a lot!

@dvdhrm dvdhrm closed this Sep 5, 2022
@Ayush1325 Ayush1325 deleted the shell-protocol branch October 11, 2022 11:19
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