Apply fixes suggested in PR

1. Limit scope to crate
2. Make std::io::error private again
3. Fix atomic ordering
4. Remove unnecessary inlines
5. Remove r-efi re-exports: Returning void pointer in place of SystemTable
   pointer now. It can be cast to SystemTable pointer of user's choice.
6. Remove std::path::Prefix::UefiDevice
7. Fix CI
8. Mimic Windows OsStrExt and OsStringExt
9. move parse_lp_cmd_line to sys_common: This allows sharing parse_lp_cmd_line
   between Windows and UEFI.
10. No longer storing path in File struct.
11. Removed Windows panic_abort instructions since they did not work for UEFI
12. Overhaul VariableBox
  a. No longer implementing Deref, DerefMut, AsRef and AsRefMut for
     VariableBox. More about why this should be avoided can be found here:
     rust-lang/unsafe-code-guidelines#256
  b. Only accessing contents of VariableBox using pointers

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

Author: Ayush1325

library/std/src/io/mod.rs

@@ -287,7 +287,7 @@ pub(crate) use error::const_io_error;
 mod buffered;
 pub(crate) mod copy;
 mod cursor;
-pub(crate) mod error;
+mod error;
 mod impls;
 pub mod prelude;
 mod readbuf;

library/std/src/os/uefi/env.rs

@@ -1,46 +1,39 @@
 //! UEFI-specific extensions to the primitives in `std::env` module
 
-use super::raw::{BootServices, RuntimeServices, SystemTable};
 use crate::ffi::c_void;
 use crate::ptr::NonNull;
 use crate::sync::atomic::{AtomicPtr, Ordering};
 
-static GLOBAL_SYSTEM_TABLE: AtomicPtr<SystemTable> = AtomicPtr::new(crate::ptr::null_mut());
-static GLOBAL_SYSTEM_HANDLE: AtomicPtr<c_void> = AtomicPtr::new(crate::ptr::null_mut());
+static GLOBAL_SYSTEM_TABLE: AtomicPtr<c_void> = AtomicPtr::new(crate::ptr::null_mut());
+static GLOBAL_IMAGE_HANDLE: AtomicPtr<c_void> = AtomicPtr::new(crate::ptr::null_mut());
 
-/// Initializes Global Atomic Pointers to SystemTable and Handle.
-/// Should only be called once in the program execution under normal circumstances.
-/// The caller should ensure that the pointers are valid.
+/// Initializes the global System Table and Image Handle pointers.
+///
+/// The standard library requires access to the UEFI System Table and the Application Image Handle
+/// to operate. Those are provided to UEFI Applications via their application entry point. By
+/// calling `init_globals()`, those pointers are retained by the standard library for future use.
+/// The pointers are never exposed to any entity outside of this application and it is guaranteed
+/// that, once the application exited, these pointers are never dereferenced again.
+///
+/// Callers are required to ensure the pointers are valid for the entire lifetime of this
+/// application. In particular, UEFI Boot Services must not be exited while an application with the
+/// standard library is loaded.
+///
+/// This function must not be called more than once.
 #[unstable(feature = "uefi_std", issue = "100499")]
-pub fn init_globals(handle: NonNull<c_void>, system_table: NonNull<SystemTable>) {
-    GLOBAL_SYSTEM_TABLE.store(system_table.as_ptr(), Ordering::SeqCst);
-    GLOBAL_SYSTEM_HANDLE.store(handle.as_ptr(), Ordering::SeqCst);
+pub unsafe fn init_globals(handle: NonNull<c_void>, system_table: NonNull<c_void>) {
+    GLOBAL_SYSTEM_TABLE.store(system_table.as_ptr(), Ordering::Release);
+    GLOBAL_IMAGE_HANDLE.store(handle.as_ptr(), Ordering::Release);
 }
 
 /// Get the SystemTable Pointer.
 #[unstable(feature = "uefi_std", issue = "100499")]
-pub fn get_system_table() -> Option<NonNull<SystemTable>> {
-    NonNull::new(GLOBAL_SYSTEM_TABLE.load(Ordering::SeqCst))
+pub fn system_table() -> Option<NonNull<c_void>> {
+    NonNull::new(GLOBAL_SYSTEM_TABLE.load(Ordering::Acquire))
 }
 
 /// Get the SystemHandle Pointer.
 #[unstable(feature = "uefi_std", issue = "100499")]
-pub fn get_system_handle() -> Option<NonNull<c_void>> {
-    NonNull::new(GLOBAL_SYSTEM_HANDLE.load(Ordering::SeqCst))
-}
-
-/// Get the BootServices Pointer.
-#[unstable(feature = "uefi_std", issue = "100499")]
-pub fn get_boot_services() -> Option<NonNull<BootServices>> {
-    let system_table = get_system_table()?;
-    let boot_services = unsafe { (*system_table.as_ptr()).boot_services };
-    NonNull::new(boot_services)
-}
-
-/// Get the RuntimeServices Pointer.
-#[unstable(feature = "uefi_std", issue = "100499")]
-pub fn get_runtime_services() -> Option<NonNull<RuntimeServices>> {
-    let system_table = get_system_table()?;
-    let runtime_services = unsafe { (*system_table.as_ptr()).runtime_services };
-    NonNull::new(runtime_services)
+pub fn image_handle() -> Option<NonNull<c_void>> {
+    NonNull::new(GLOBAL_IMAGE_HANDLE.load(Ordering::Acquire))
 }

library/std/src/os/uefi/ffi.rs

@@ -3,31 +3,33 @@ use crate::sealed::Sealed;
 use crate::sys_common::ucs2;
 
 #[unstable(feature = "uefi_std", issue = "100499")]
-pub trait OsStrExt: Sealed {
-    /// This function does not do any allocation
-    fn to_ucs2<'a>(&'a self) -> ucs2::EncodeUcs2<'a>;
+pub use ucs2::EncodeUcs2;
 
-    /// Creates a UCS-2 Vec which can be passed for FFI.
-    /// Note: This function will replace `NULL` and other characters which are not valid in UEFI
-    /// strings with `std::sys_common::ucs::Ucs2Char::REPLACEMENT_CHARACTER`
-    fn to_ffi_string(&self) -> Vec<u16> {
-        let mut v: Vec<u16> = self
-            .to_ucs2()
-            .map(|x| match x {
-                Ok(c) => c,
-                Err(_) => ucs2::Ucs2Char::REPLACEMENT_CHARACTER,
-            })
-            .map(u16::from)
-            .collect();
-        v.push(0);
-        v.shrink_to_fit();
-        v
-    }
+#[unstable(feature = "uefi_std", issue = "100499")]
+pub trait OsStrExt: Sealed {
+    /// Re-encodes an `OsStr` as a wide character sequence, i.e., UCS-2
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use std::ffi::OsString;
+    /// use std::os::uefi::ffi::*;
+    ///
+    /// // UTF-2 encoding for "Unicode".
+    /// let source = [0x0055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065];
+    ///
+    /// let string = OsString::from_wide(&source[..]);
+    ///
+    /// let result: Vec<u16> = string.encode_wide().collect();
+    /// assert_eq!(&source[..], &result[..]);
+    /// ```
+    fn encode_wide<'a>(&'a self) -> EncodeUcs2<'a>;
 }
 
 impl OsStrExt for OsStr {
-    fn to_ucs2<'a>(&'a self) -> ucs2::EncodeUcs2<'a> {
-        // This conversion should never fail since the underlying OsStr is UTF-8 encoded
+    fn encode_wide<'a>(&'a self) -> EncodeUcs2<'a> {
+        // SAFETY: Calling unwrap on `self.to_str` is safe since the underlying OsStr is UTF-8
+        // encoded
         ucs2::EncodeUcs2::from_str(self.to_str().unwrap())
     }
 }
@@ -37,26 +39,23 @@ pub trait OsStringExt: Sealed
 where
     Self: Sized,
 {
-    fn from_ucs2_lossy(ucs: &[u16]) -> Self;
-
-    // For Null terminated UCS-2 Strings.
-    #[inline]
-    fn from_ucs2_null_termintated(ucs: &[u16]) -> Self {
-        Self::from_ucs2_lossy(&ucs[..(ucs.len() - 1)])
-    }
-
-    // Create OsString from an FFI obtained pointer.
-    // Len is the number of elemented in the string, not number of bytes.
-    // Note: This string is assumed to be null terminated
-    #[inline]
-    unsafe fn from_ffi(ucs: *mut u16, len: usize) -> Self {
-        let s = crate::slice::from_raw_parts(ucs, len);
-        Self::from_ucs2_null_termintated(s)
-    }
+    /// Creates an `OsString` from a UCS-2 slice of 16-bit code units.
+    /// # Examples
+    ///
+    /// ```
+    /// use std::ffi::OsString;
+    /// use std::os::uefi::ffi::*;
+    ///
+    /// // UTF-16 encoding for "Unicode".
+    /// let source = [0x0055, 0x006E, 0x0069, 0x0063, 0x006F, 0x0064, 0x0065];
+    ///
+    /// let string = OsString::from_wide(&source[..]);
+    /// ```
+    fn from_wide(ucs: &[u16]) -> Self;
 }
 
 impl OsStringExt for OsString {
-    fn from_ucs2_lossy(ucs: &[u16]) -> Self {
+    fn from_wide(ucs: &[u16]) -> Self {
         // Min capacity(in case of all ASCII) is `ucs.len()`
         let mut buf = String::with_capacity(ucs.len());
 

library/std/src/os/uefi/mod.rs

@@ -4,4 +4,3 @@
 
 pub mod env;
 pub mod ffi;
-pub mod raw;

library/std/src/os/uefi/raw.rs

@@ -186,11 +186,6 @@ pub enum Prefix<'a> {
     /// Prefix `C:` for the given disk drive.
     #[stable(feature = "rust1", since = "1.0.0")]
     Disk(#[stable(feature = "rust1", since = "1.0.0")] u8),
-
-    /// UEFI Device Prefix. e.g., `PciRoot(0x0)/Pci(0x1,0x1)/Ata(0x0)/CDROM(0x0)`
-    /// Sometimes also represented like Windows Disks, but this is more general
-    #[unstable(feature = "uefi_std", issue = "100499")]
-    UefiDevice(&'a OsStr),
 }
 
 impl<'a> Prefix<'a> {
@@ -209,7 +204,6 @@ impl<'a> Prefix<'a> {
             UNC(x, y) => 2 + os_str_len(x) + if os_str_len(y) > 0 { 1 + os_str_len(y) } else { 0 },
             DeviceNS(x) => 4 + os_str_len(x),
             Disk(_) => 2,
-            UefiDevice(x) => os_str_len(x),
         }
     }
 
@@ -2165,7 +2159,7 @@ impl Path {
         !self.is_absolute()
     }
 
-    pub(crate) fn prefix(&self) -> Option<Prefix<'_>> {
+    fn prefix(&self) -> Option<Prefix<'_>> {
         self.components().prefix
     }
 

library/std/src/path.rs

@@ -3,11 +3,11 @@
 //! Takes a lot of inspiration from Windows allocator for Alignment > 8.
 
 use crate::alloc::{GlobalAlloc, Layout, System};
-use crate::os::uefi;
+use crate::ptr;
+use crate::sys::uefi::common;
 
 pub(crate) const POOL_ALIGNMENT: usize = 8;
-// FIXME: Maybe allow chaing the MEMORY_TYPE. However, since allocation is done even before main,
-// there will be a few allocations with the default MEMORY_TYPE.
+
 const MEMORY_TYPE: u32 = r_efi::efi::LOADER_DATA;
 
 #[stable(feature = "alloc_system_type", since = "1.28.0")]
@@ -16,46 +16,39 @@ unsafe impl GlobalAlloc for System {
         let align = layout.align();
         let size = layout.size();
 
-        // Return NULL pointer if `layout.size == 0`
-        if size == 0 {
-            return core::ptr::null_mut();
-        }
-
         // Return NULL pointer if boot_services pointer cannot be obtained. The only time this
         // should happen is if SystemTable has not been initialized
-        let boot_services = match uefi::env::get_boot_services() {
+        let boot_services = match common::get_boot_services() {
             Some(x) => x,
-            None => return core::ptr::null_mut(),
+            None => return ptr::null_mut(),
         };
 
         let allocate_pool_ptr = unsafe { (*boot_services.as_ptr()).allocate_pool };
 
-        let mut ptr: *mut crate::ffi::c_void = crate::ptr::null_mut();
+        let mut ptr: *mut crate::ffi::c_void = ptr::null_mut();
         let aligned_size = align_size(size, align);
 
         let r = (allocate_pool_ptr)(MEMORY_TYPE, aligned_size, &mut ptr);
 
         if r.is_error() || ptr.is_null() {
-            return crate::ptr::null_mut();
+            return ptr::null_mut();
         }
 
         unsafe { align_ptr(ptr.cast(), align) }
     }
 
     unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
-        if layout.size() != 0 {
-            let boot_services = match uefi::env::get_boot_services() {
-                Some(x) => x,
-                None => return,
-            };
+        let boot_services = match common::get_boot_services() {
+            Some(x) => x,
+            None => return,
+        };
 
-            let free_pool_ptr = unsafe { (*boot_services.as_ptr()).free_pool };
+        let free_pool_ptr = unsafe { (*boot_services.as_ptr()).free_pool };
 
-            let ptr = unsafe { unalign_ptr(ptr, layout.align()) };
-            let r = (free_pool_ptr)(ptr.cast());
+        let ptr = unsafe { unalign_ptr(ptr, layout.align()) };
+        let r = (free_pool_ptr)(ptr.cast());
 
-            assert!(!r.is_error());
-        }
+        assert!(!r.is_error());
     }
 }