Fix/robustness

.github/workflows/ci.yml

@@ -0,0 +1,18 @@
+name: CI
+
+on:
+  push:
+    branches: [master, main]
+  pull_request:
+    branches: [master, main]
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: dtolnay/rust-toolchain@stable
+      - run: cargo fmt --check
+      - run: cargo clippy -- -D warnings
+      - run: cargo build
+      - run: cargo test

Cargo.toml

@@ -1,14 +1,11 @@
 [package]
 name = "hugepage-rs"
 version = "0.1.0"
-edition = "2018"
+edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "wrapped allocator for linux hugepage"
 repository = "https://github.com/cppcoffee/hugepage-rs"
 readme = "README.md"
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
-
 [dependencies]
-lazy_static = "1.4"
 libc = "0.2"

src/allocator.rs

@@ -6,64 +6,51 @@ use std::{
     fs::File,
     io::Read,
     ptr::null_mut,
+    sync::LazyLock,
 };
 
 // https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt
-//
-// The output of "cat /proc/meminfo" will include lines like:
-// ...
-// HugePages_Total: uuu
-// HugePages_Free:  vvv
-// HugePages_Rsvd:  www
-// HugePages_Surp:  xxx
-// Hugepagesize:    yyy kB
-// Hugetlb:         zzz kB
-
-// constant.
 const MEMINFO_PATH: &str = "/proc/meminfo";
 const TOKEN: &str = "Hugepagesize:";
 
-lazy_static! {
-    static ref HUGEPAGE_SIZE: isize = {
-        let buf = File::open(MEMINFO_PATH).map_or("".to_owned(), |mut f| {
-            let mut s = String::new();
-            let _ = f.read_to_string(&mut s);
-            s
-        });
-        parse_hugepage_size(&buf)
-    };
-}
+pub(crate) static HUGEPAGE_SIZE: LazyLock<usize> = LazyLock::new(|| {
+    let mut buf = String::new();
+    if let Ok(mut f) = File::open(MEMINFO_PATH) {
+        let _ = f.read_to_string(&mut buf);
+    }
+    parse_hugepage_size(&buf).expect("failed to parse hugepage size from /proc/meminfo")
+});
 
-fn parse_hugepage_size(s: &str) -> isize {
+fn parse_hugepage_size(s: &str) -> Option<usize> {
     for line in s.lines() {
         if line.starts_with(TOKEN) {
             let mut parts = line[TOKEN.len()..].split_whitespace();
-
-            let p = parts.next().unwrap_or("0");
-            let mut hugepage_size = p.parse::<isize>().unwrap_or(-1);
-
-            hugepage_size *= parts.next().map_or(1, |x| match x {
-                "kB" => 1024,
-                _ => 1,
-            });
-
-            return hugepage_size;
+            let size: usize = parts.next()?.parse().ok()?;
+            let multiplier = match parts.next() {
+                Some("kB") => 1024,
+                Some(_) => return None,
+                None => 1,
+            };
+            return Some(size * multiplier);
         }
     }
-
-    return -1;
+    None
 }
 
 fn align_to(size: usize, align: usize) -> usize {
     (size + align - 1) & !(align - 1)
 }
 
+pub(crate) fn aligned_size(layout: Layout) -> usize {
+    align_to(layout.size(), *HUGEPAGE_SIZE)
+}
+
 // hugepage allocator.
 pub(crate) struct HugePageAllocator;
 
 unsafe impl GlobalAlloc for HugePageAllocator {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
-        let len = align_to(layout.size(), *HUGEPAGE_SIZE as usize);
+        let len = aligned_size(layout);
         let p = libc::mmap(
             null_mut(),
             len,
@@ -81,7 +68,8 @@ unsafe impl GlobalAlloc for HugePageAllocator {
     }
 
     unsafe fn dealloc(&self, p: *mut u8, layout: Layout) {
-        libc::munmap(p as *mut c_void, layout.size());
+        let len = aligned_size(layout);
+        libc::munmap(p as *mut c_void, len);
     }
 }
 
@@ -92,13 +80,11 @@ mod tests {
 
     #[test]
     fn test_parse_hugepage_size() {
-        // correct.
-        assert_eq!(parse_hugepage_size("Hugepagesize:1024"), 1024);
-        assert_eq!(parse_hugepage_size("Hugepagesize: 2 kB"), 2048);
-
-        // wrong.
-        assert_eq!(parse_hugepage_size("Hugepagesize:1kB"), -1);
-        assert_eq!(parse_hugepage_size("Hugepagesize: 2kB"), -1);
+        assert_eq!(parse_hugepage_size("Hugepagesize: 2048 kB"), Some(2048 * 1024));
+        assert_eq!(parse_hugepage_size("Hugepagesize: 1024"), Some(1024));
+        assert_eq!(parse_hugepage_size("Hugepagesize:"), None);
+        assert_eq!(parse_hugepage_size("Hugepagesize: abc kB"), None);
+        assert_eq!(parse_hugepage_size(""), None);
     }
 
     #[test]
@@ -111,7 +97,6 @@ mod tests {
     fn test_allocator() {
         let hugepage_alloc = HugePageAllocator;
 
-        // u16.
         unsafe {
             let layout = Layout::new::<u16>();
             let p = hugepage_alloc.alloc(layout);
@@ -121,7 +106,6 @@ mod tests {
             hugepage_alloc.dealloc(p, layout);
         }
 
-        // array.
         unsafe {
             let layout = Layout::array::<char>(2048).unwrap();
             let dst = hugepage_alloc.alloc(layout);

src/boxed.rs

@@ -2,35 +2,38 @@ use crate::default_allocator;
 
 use std::alloc::{GlobalAlloc, Layout};
 use std::ops::{Deref, DerefMut};
-use std::ptr::NonNull;
+use std::ptr::{self, NonNull};
 
 /// A pointer type for hugepage allocation.
-///
-/// Allocates memory on the hugepage and then places x into it.
 pub struct Box<T> {
     data: NonNull<T>,
 }
 
+unsafe impl<T: Send> Send for Box<T> {}
+unsafe impl<T: Sync> Sync for Box<T> {}
+
 impl<T> Box<T> {
     pub fn new(data: T) -> Box<T> {
         let layout = Layout::new::<T>();
         unsafe {
-            let mut p = NonNull::new(default_allocator().alloc(layout) as *mut T).unwrap();
-            *(p.as_mut()) = data;
-            Self { data: p }
+            let p = default_allocator().alloc(layout) as *mut T;
+            let mut nn = NonNull::new(p).expect("hugepage allocation failed");
+            ptr::write(nn.as_mut(), data);
+            Self { data: nn }
         }
     }
 
     pub unsafe fn from_raw(raw: *mut T) -> Self {
         Self {
-            data: NonNull::new(raw).unwrap(),
+            data: NonNull::new(raw).expect("Box::from_raw received null pointer"),
         }
     }
 }
 
 impl<T> Drop for Box<T> {
     fn drop(&mut self) {
         unsafe {
+            ptr::drop_in_place(self.data.as_ptr());
             default_allocator().dealloc(self.data.as_ptr() as *mut u8, Layout::new::<T>());
         }
     }
@@ -55,14 +58,12 @@ mod tests {
 
     #[test]
     fn test_boxed() {
-        // variable.
         {
             let mut v = Box::new(5);
             *v += 42;
             assert_eq!(*v, 47);
         }
 
-        // array.
         {
             let src: [u32; 4] = [1, 2, 3, 4];
             let mut v = Box::new(src);
@@ -73,4 +74,13 @@ mod tests {
             assert_eq!(&*v, &[2, 2, 3, 4]);
         }
     }
+
+    #[test]
+    fn test_boxed_drop_inner() {
+        // Verify that inner values with Drop are properly dropped
+        let s = String::from("hello hugepage");
+        let b = Box::new(s.clone());
+        assert_eq!(&*b, &s);
+        drop(b); // should not leak
+    }
 }

src/lib.rs

@@ -1,7 +1,3 @@
-#[cfg(target_os = "linux")]
-#[macro_use]
-extern crate lazy_static;
-
 #[cfg(target_os = "linux")]
 use std::alloc::{GlobalAlloc, Layout};
 
@@ -16,9 +12,7 @@ mod boxed;
 pub use boxed::Box;
 
 #[cfg(target_os = "linux")]
-lazy_static! {
-    static ref HUGEPAGE_ALLOCATOR: HugePageAllocator = HugePageAllocator;
-}
+static HUGEPAGE_ALLOCATOR: HugePageAllocator = HugePageAllocator;
 
 #[cfg(target_os = "linux")]
 pub(crate) fn default_allocator() -> &'static HugePageAllocator {
@@ -27,44 +21,21 @@ pub(crate) fn default_allocator() -> &'static HugePageAllocator {
 
 /// Allocate memory with the hugepage allocator.
 ///
-/// This function forwards calls to the HugePageAllocator.alloc() function.
-///
 /// # Safety
 ///
 /// See [`GlobalAlloc::alloc`].
-///
-/// # Examples
-///
-/// ```
-/// use std::alloc::Layout;
-/// use hugepage_rs::{alloc, dealloc};
-///
-/// unsafe {
-///     let layout = Layout::new::<u16>();
-///     let ptr = alloc(layout);
-///
-///     *(ptr as *mut u16) = 42;
-///     assert_eq!(*(ptr as *mut u16), 42);
-///
-///     dealloc(ptr, layout);
-/// }
-/// ```
 #[cfg(target_os = "linux")]
 pub fn alloc(layout: Layout) -> *mut u8 {
     unsafe { HUGEPAGE_ALLOCATOR.alloc(layout) }
 }
 
 /// Deallocate memory with the hugepage allocator.
 ///
-/// This function forwards calls to the HugePageAllocator.dealloc() function.
-///
 /// # Safety
 ///
-/// This function is unsafe because undefined behavior can result if the caller does not ensure all of the following:
-///
-/// - ptr must denote a block of memory currently allocated via this allocator,
-/// - layout must be the same layout that was used to allocate that block of memory.
+/// - `ptr` must denote a block of memory currently allocated via this allocator.
+/// - `layout` must be the same layout that was used to allocate that block of memory.
 #[cfg(target_os = "linux")]
-pub fn dealloc(p: *mut u8, layout: Layout) {
-    unsafe { HUGEPAGE_ALLOCATOR.dealloc(p, layout) }
+pub unsafe fn dealloc(ptr: *mut u8, layout: Layout) {
+    HUGEPAGE_ALLOCATOR.dealloc(ptr, layout)
 }