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

Commit d23e994

Browse files
committed
uefi-raw: net: covert union to normal type
The union makes some things hard to work with, and we can simply remove it. We get multiple advantages from that: - no more chance to have uninitialized bytes - We can `Debug`-print the type - Some implementations become simpler
1 parent e03ebda commit d23e994

File tree

2 files changed

+46
-72
lines changed

2 files changed

+46
-72
lines changed

‎uefi-raw/src/net.rs

Lines changed: 44 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@
2828
//! - `[u8; 6]` -> [`MacAddress`]
2929
//! - `[u8; 32]` -> [`MacAddress`]
3030
31-
use core::fmt::{Debug, Formatter};
3231
use core::net::{IpAddr as StdIpAddr, Ipv4Addr as StdIpv4Addr, Ipv6Addr as StdIpv6Addr};
33-
use core::{fmt, mem};
3432

3533
/// An IPv4 internet protocol address.
3634
///
@@ -92,38 +90,27 @@ impl From<Ipv6Address> for StdIpv6Addr {
9290
}
9391
}
9492

95-
/// EFI ABI-compatible union of an IPv4 or IPv6 internet protocol address.
96-
///
97-
/// Corresponds to the `EFI_IP_ADDRESS` type in the UEFI specification. This
98-
/// type is defined in the same way as edk2 for compatibility with C code. Note
99-
/// that this is an **untagged union**, so there's no way to tell which type of
100-
/// address an `IpAddress` value contains without additional context.
93+
/// EFI ABI-compatible union of an IPv4 or IPv6 internet protocol address,
94+
/// corresponding to `EFI_IP_ADDRESS` type in the UEFI specification.
10195
///
10296
/// See the [module documentation] to get an overview over the relation to the
10397
/// types from [`core::net`].
10498
///
99+
/// # UEFI Information
100+
/// Corresponds to the `EFI_IP_ADDRESS` type in the UEFI specification. Instead
101+
/// of using an untagged C union, we use this more rusty type. ABI-wise it is
102+
/// the same but less cumbersome to work with in Rust.
103+
///
105104
/// [module documentation]: self
106-
#[derive(Clone, Copy)]
107-
#[repr(C)]
108-
pub union IpAddress {
109-
/// An IPv4 internet protocol address.
110-
pub v4: Ipv4Address,
111-
112-
/// An IPv6 internet protocol address.
113-
pub v6: Ipv6Address,
114-
115-
/// This member serves to align the whole type to 4 bytes as required by
116-
/// the spec. Note that this is slightly different from `repr(align(4))`,
117-
/// which would prevent placing this type in a packed structure.
118-
align_helper: [u32; 4],
119-
}
105+
#[derive(Debug, Clone, Copy)]
106+
#[repr(C, align(4))]
107+
pub struct IpAddress(pub [u8; 16]);
120108

121109
impl IpAddress {
122110
/// Construct a new zeroed address.
123111
#[must_use]
124112
pub const fn new_zeroed() -> Self {
125-
// SAFETY: All bit patterns are valid.
126-
unsafe { mem::zeroed() }
113+
Self([0; 16])
127114
}
128115

129116
/// Construct a new IPv4 address.
@@ -132,10 +119,9 @@ impl IpAddress {
132119
/// is needed.
133120
#[must_use]
134121
pub const fn new_v4(octets: [u8; 4]) -> Self {
135-
// Initialize all bytes to zero first.
136-
let mut obj = Self::new_zeroed();
137-
obj.v4 = Ipv4Address(octets);
138-
obj
122+
Self([
123+
octets[0], octets[1], octets[2], octets[3], 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
124+
])
139125
}
140126

141127
/// Construct a new IPv6 address.
@@ -144,17 +130,14 @@ impl IpAddress {
144130
/// is needed.
145131
#[must_use]
146132
pub const fn new_v6(octets: [u8; 16]) -> Self {
147-
// Initialize all bytes to zero first.
148-
let mut obj = Self::new_zeroed();
149-
obj.v6 = Ipv6Address(octets);
150-
obj
133+
Self(octets)
151134
}
152135

153136
/// Returns the octets of the union. Without additional context, it is not
154137
/// clear whether the octets represent an IPv4 or IPv6 address.
155138
#[must_use]
156139
pub const fn octets(&self) -> [u8; 16] {
157-
unsafe{self.v6.octets()}
140+
self.0
158141
}
159142

160143
/// Returns a raw pointer to the IP address.
@@ -181,11 +164,12 @@ impl IpAddress {
181164
#[must_use]
182165
pub fn to_std_ip_addr(self, is_ipv6: bool) -> StdIpAddr {
183166
if is_ipv6 {
184-
StdIpAddr::V6(StdIpv6Addr::from(unsafe{self.v6.octets()}))
167+
StdIpAddr::V6(StdIpv6Addr::from(self.octets()))
185168
} else {
186169
let has_extra_bytes = self.octets()[4..].iter().any(|&x| x != 0);
187170
assert!(!has_extra_bytes);
188-
StdIpAddr::V4(StdIpv4Addr::from(unsafe { self.v4.octets() }))
171+
let octets: [u8; 4] = self.octets()[..4].try_into().unwrap();
172+
StdIpAddr::V4(StdIpv4Addr::from(octets))
189173
}
190174
}
191175

@@ -212,20 +196,7 @@ impl IpAddress {
212196
/// additional context that the IP is indeed an IPv6 address.
213197
#[must_use]
214198
pub unsafe fn as_ipv6(&self) -> Ipv6Address {
215-
Ipv6Address::from(unsafe { self.v6.octets() })
216-
}
217-
}
218-
219-
impl Debug for IpAddress {
220-
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
221-
f.debug_struct("IpAddress")
222-
// SAFETY: All constructors ensure that all bytes are always
223-
// initialized.
224-
.field("v4", &unsafe { self.v4 })
225-
// SAFETY: All constructors ensure that all bytes are always
226-
// initialized.
227-
.field("v6", &unsafe { self.v6 })
228-
.finish()
199+
Ipv6Address::from(self.octets())
229200
}
230201
}
231202

@@ -335,6 +306,18 @@ mod tests {
335306
101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116,
336307
];
337308

309+
/// Ensures ABI-compatibility, as described here:
310+
/// <https://github.com/tianocore/edk2/blob/b1887152024c7eb0cc7de735b0b57febd6099bf9/MdePkg/Include/Uefi/UefiBaseType.h#L100>
311+
#[test]
312+
fn test_abi() {
313+
assert_eq!(size_of::<IpAddress>(), 16);
314+
assert_eq!(align_of::<IpAddress>(), 4);
315+
assert_eq!(size_of::<Ipv4Address>(), 4);
316+
assert_eq!(align_of::<Ipv6Address>(), 1);
317+
assert_eq!(size_of::<Ipv6Address>(), 16);
318+
assert_eq!(align_of::<Ipv6Address>(), 1);
319+
}
320+
338321
/// Test round-trip conversion between `Ipv4Address` and `StdIpv4Addr`.
339322
#[test]
340323
fn test_ip_addr4_conversion() {
@@ -358,11 +341,14 @@ mod tests {
358341
fn test_ip_addr_conversion() {
359342
let core_addr = StdIpAddr::V4(StdIpv4Addr::from(TEST_IPV4));
360343
let uefi_addr = IpAddress::from(core_addr);
361-
assert_eq!(unsafe { uefi_addr.v4.0 }, TEST_IPV4);
344+
assert_eq!(
345+
unsafe { uefi_addr.try_as_ipv4().unwrap() }.octets(),
346+
TEST_IPV4
347+
);
362348

363349
let core_addr = StdIpAddr::V6(StdIpv6Addr::from(TEST_IPV6));
364350
let uefi_addr = IpAddress::from(core_addr);
365-
assert_eq!(unsafe { uefi_addr.v6.0}, TEST_IPV6);
351+
assert_eq!(unsafe { uefi_addr.as_ipv6()}.octets(), TEST_IPV6);
366352
}
367353

368354
/// Tests the From-impls as described in the module description.
@@ -412,30 +398,18 @@ mod tests {
412398
}
413399
}
414400

415-
/// Tests that all bytes are initialized and that the Debug print doesn't
416-
/// produce errors, when Miri executes this.
417-
#[test]
418-
fn test_ip_address_debug_memory_safe() {
419-
let uefi_addr = IpAddress::new_v6(TEST_IPV6);
420-
std::eprintln!("{uefi_addr:#?}");
421-
}
422-
423401
/// Tests the expected flow of types in a higher-level UEFI API.
424402
#[test]
425403
fn test_uefi_flow() {
426404
fn efi_retrieve_efi_ip_addr(addr: &mut IpAddress, is_ipv6: bool) {
427-
// SAFETY: Alignment is guaranteed and memory is initialized.
428-
unsafe {
429-
addr.v4.0[0] = 42;
430-
addr.v4.0[1] = 42;
431-
addr.v4.0[2] = 42;
432-
addr.v4.0[3] = 42;
433-
}
405+
addr.0[0] = 42;
406+
addr.0[1] = 42;
407+
addr.0[2] = 42;
408+
addr.0[3] = 42;
409+
434410
if is_ipv6 {
435-
unsafe {
436-
addr.v6.0[14] = 42;
437-
addr.v6.0[15] = 42;
438-
}
411+
addr.0[14] = 42;
412+
addr.0[15] = 42;
439413
}
440414
}
441415

‎uefi/src/proto/network/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ impl IpAddress {
4444
#[must_use]
4545
const unsafe fn from_raw(ip_addr: uefi_raw::net::IpAddress, is_ipv6: bool) -> Self {
4646
if is_ipv6 {
47-
Self::new_v6(unsafe{ip_addr.v6.0})
47+
Self::new_v6(ip_addr.0)
4848
} else {
49-
Self::new_v4(unsafe{ ip_addr.v4.0})
49+
Self::new_v4([ip_addr.0[0], ip_addr.0[1], ip_addr.0[2], ip_addr.0[3]])
5050
}
5151
}
5252

0 commit comments

Comments
(0)

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