Rename boot services' memset to set_mem
#235
Conversation
In boot.rs we see there is an unsafe function named memset. This function has a signature different than memset(3), but has a name different than that in the edk2 sources. This is confusing and will lead to bugs in a very unsafe function when people mix up the order of the value and the size. We should either name the function memset and give it the libc-style signature memset(&self, buffer: *mut u8, value: u8, size: usize), or name the function set_mem and give it the signature from EFI_SET_MEM. uefi-rs/src/table/boot.rs Line 529 in 948464c pub unsafe fn memset(&self, buffer: *mut u8, size: usize, value: u8) { See rust-osdev#232
|
Note that I made this PR entirely through the web UI to see how useful that workflow is, and have not run the tests locally. I am relying on travis CI to catch bugs. |
|
Also, I have elected to rename the function set_mem. It's possible to hardcode two integers in both the value and size arguments of the old memset which would be wrong but still valid. I.e. If we rename the function this silent breakage can't happen. |
|
@iankronquist I think some tests failed since they have to be updated to the function's new API. |
|
I think I fixed it but Travis doesn't seem to be checking in with GitHub. This new GitHub checks feature seems more brittle than it was a few years ago? |
Yeah, it's a minor security change GitHub made 😅 the CI doesn't run automatically for first time contributors of a project, I have to manually approve running the flows after you make some changes. |
GabrielMajeri
changed the title
memset to set_mem
|
Thanks for contributing. I've pulled the latest changes from |
In boot.rs we see there is an unsafe function named memset. This function has a signature different than memset(3), but has a name different than that in the edk2 sources. This is confusing and will lead to bugs in a very unsafe function when people mix up the order of the value and the size.
We should name the function set_mem and give it the signature from EFI_SET_MEM.
uefi-rs/src/table/boot.rs
Line 529 in 948464c
See #232