Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

uefi-raw: move types to net module #1747

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

Merged
nicholasbishop merged 1 commit into main from foobar
Aug 24, 2025
Merged

uefi-raw: move types to net module #1747

nicholasbishop merged 1 commit into main from foobar
Aug 24, 2025

Conversation

Copy link
Member

@phip1611 phip1611 commented Aug 22, 2025

Split-out from #1699.

We have enough network-related types to justify
a dedicated module:

  • IpAddress
  • Ipv4Address
  • Ipv6Address
  • MacAddress

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

Copy link
Member

There are a few unexpected changes:

diff --git a/uefi-raw/src/net.rs b/uefi-raw/src/net.rs
index 3b4f3e5f..71662aef 100644
--- a/uefi-raw/src/net.rs
+++ b/uefi-raw/src/net.rs
@@ -43,7 +43,7 @@ impl From<Ipv6Address> for core::net::Ipv6Addr {
 ///
 /// Corresponds to the `EFI_IP_ADDRESS` type in the UEFI specification. This
 /// type is defined in the same way as edk2 for compatibility with C code. Note
-/// that this is an **untagged union**, so there's no way to tell which type of
+/// that this is an untagged union, so there's no way to tell which type of
 /// address an `IpAddress` value contains without additional context.
 #[derive(Clone, Copy)]
 #[repr(C)]
@@ -106,15 +106,7 @@ impl From<core::net::IpAddr> for IpAddress {
 }
 }
 
-/// UEFI Media Access Control (MAC) address.
-///
-/// UEFI supports multiple network protocols and hardware types, not just
-/// Ethernet. Some of them may use MAC addresses longer than 6 bytes. To be
-/// protocol-agnostic and future-proof, the UEFI spec chooses a maximum size
-/// that can hold any supported media access control address.
-///
-/// In most cases, this is just a typical `[u8; 6]` Ethernet style MAC
-/// address with the rest of the bytes being zero.
+/// A Media Access Control (MAC) address.
 #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[repr(transparent)]
 pub struct MacAddress(pub [u8; 32]);
@@ -122,7 +114,12 @@ pub struct MacAddress(pub [u8; 32]);
 impl From<[u8; 6]> for MacAddress {
 fn from(octets: [u8; 6]) -> Self {
 let mut buffer = [0; 32];
- buffer.copy_from_slice(&octets);
+ buffer[0] = octets[0];
+ buffer[1] = octets[1];
+ buffer[2] = octets[2];
+ buffer[3] = octets[3];
+ buffer[4] = octets[4];
+ buffer[5] = octets[5];
 Self(buffer)
 }
 }

In particular, was dropping part of the MacAddress docstring intentional? The copy_from_slice change looks like a bugfix, since the slice lengths are different. Might be better to do that as buffer[..6].copy_from_slice(&octets);, but either way it ideally wouldn't be mixed into the same commit that splits out the module, or at least call it out in the commit message.

Copy link
Member Author

phip1611 commented Aug 22, 2025
edited
Loading

Yikes. I was certain I fixed this already 🤔 I work from two different laptops - perhaps I fixed it on one without pushing and continued work on another - anyway, thanks for pointing that out! I'll fix it.

nicholasbishop reacted with thumbs up emoji

We have enough network-related types to justify
a dedicated module:
- IpAddress
- Ipv4Address
- Ipv6Address
- MacAddress
Copy link
Member Author

Okay, I think it should be fixed now. Thanks for the close attention on this one, good catch

@phip1611 phip1611 self-assigned this Aug 23, 2025
@nicholasbishop nicholasbishop added this pull request to the merge queue Aug 24, 2025
Merged via the queue into main with commit 1fc781b Aug 24, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicholasbishop nicholasbishop nicholasbishop approved these changes

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /