Addressing reviewer comments

src/dlmalloc.rs

@@ -7,7 +7,7 @@ use core::cmp;
 use core::mem;
 use core::ptr;
 
-use System;
+use Allocator;
 
 pub struct Dlmalloc<S> {
     smallmap: u32,
@@ -113,7 +113,7 @@ impl<Sys> Dlmalloc<Sys> {
     }
 }
 
-impl<Sys: System> Dlmalloc<Sys> {
+impl<Sys: Allocator> Dlmalloc<Sys> {
     // TODO: can we get rid of this?
     pub fn malloc_alignment(&self) -> usize {
         mem::size_of::<usize>() * 2
@@ -1761,7 +1761,7 @@ impl Segment {
         (*seg).flags & EXTERN != 0
     }
 
-    unsafe fn can_release_part<S: System>(system_allocator: &S, seg: *mut Segment) -> bool {
+    unsafe fn can_release_part<A: Allocator>(system_allocator: &A, seg: *mut Segment) -> bool {
         system_allocator.can_release_part((*seg).flags >> 1)
     }
 
@@ -1781,11 +1781,11 @@ impl Segment {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use Platform;
+    use System;
 
     // Prime the allocator with some allocations such that there will be free
     // chunks in the treemap
-    unsafe fn setup_treemap<S: System>(a: &mut Dlmalloc<S>) {
+    unsafe fn setup_treemap<A: Allocator>(a: &mut Dlmalloc<A>) {
         let large_request_size = NSMALLBINS * (1 << SMALLBIN_SHIFT);
         assert!(!a.is_small(large_request_size));
         let large_request1 = a.malloc(large_request_size);
@@ -1799,9 +1799,8 @@ mod tests {
     #[test]
     // Test allocating, with a non-empty treemap, a specific size that used to
     // trigger an integer overflow bug
-    #[cfg(any(target_os = "linux", target_arch = "wasm32", target_os = "macos"))]
     fn treemap_alloc_overflow_minimal() {
-        let mut a: Dlmalloc<Platform> = Dlmalloc::new(Platform::new());
+        let mut a = Dlmalloc::new(System::new());
         unsafe {
             setup_treemap(&mut a);
             let min_idx31_size = (0xc000 << TREEBIN_SHIFT) - a.chunk_overhead() + 1;
@@ -1810,10 +1809,9 @@ mod tests {
     }
 
     #[test]
-    #[cfg(any(target_os = "linux", target_arch = "wasm32", target_os = "macos"))]
     // Test allocating the maximum request size with a non-empty treemap
     fn treemap_alloc_max() {
-        let mut a: Dlmalloc<Platform> = Dlmalloc::new(Platform::new());
+        let mut a = Dlmalloc::new(System::new());
         unsafe {
             setup_treemap(&mut a);
             let max_request_size = a.max_request() - 1;

src/dummy.rs

@@ -0,0 +1,42 @@
+use core::ptr;
+use Allocator;
+
+pub struct System {
+    _priv: (),
+}
+
+impl System {
+    const fn new() -> System {
+        System { _priv: () }
+    }
+}
+
+unsafe impl Allocator for System {
+    fn alloc(&self, size: usize) -> (*mut u8, usize, u32) {
+        (ptr::null_mut(), 0, 0)
+    }
+
+    fn remap(&self, ptr: *mut u8, oldsize: usize, newsize: usize, can_move: bool) -> *mut u8 {
+        ptr::null_mut()
+    }
+
+    fn free_part(&self, ptr: *mut u8, oldsize: usize, newsize: usize) -> bool {
+        false
+    }
+
+    fn free(&self, ptr: *mut u8, size: usize) -> bool {
+        false
+    }
+
+    fn can_release_part(&self, flags: u32) -> bool {
+        false
+    }
+
+    fn allocates_zeros(&self) -> bool {
+        false
+    }
+
+    fn page_size(&self) -> usize {
+        1
+    }
+}

src/lib.rs

@@ -20,8 +20,7 @@
 use core::alloc::{Alloc, AllocErr, Layout};
 use core::cmp;
 use core::ptr;
-#[cfg(any(target_os = "linux", target_os = "macos", target_arch = "wasm32"))]
-use sys::Platform;
+use sys::System;
 
 #[cfg(all(feature = "global", not(test)))]
 pub use self::global::GlobalDlmalloc;
@@ -30,36 +29,41 @@ mod dlmalloc;
 #[cfg(all(feature = "global", not(test)))]
 mod global;
 
-/// A platform interface
-pub unsafe trait System: Send {
+/// A system interface
+pub unsafe trait Allocator: Send {
     /// Allocates system memory region of at least `size` bytes
     /// Returns a triple of `(base, size, flags)` where `base` is a pointer to the beginning of the
     /// allocated memory region. `size` is the actual size of the region while `flags` specifies
     /// properties of the allocated region. If `EXTERN_BIT` (bit 0) set in flags, then we did not
     /// allocate this segment and so should not try to deallocate or merge with others.
+    /// This function can return a `std::ptr::null_mut()` when allocation fails (other values of
+    /// the triple will be ignored).
     fn alloc(&self, size: usize) -> (*mut u8, usize, u32);
 
     /// Remaps system memory region at `ptr` with size `oldsize` to a potential new location with
     /// size `newsize`. `can_move` indicates if the location is allowed to move to a completely new
     /// location, or that it is only allowed to change in size. Returns a pointer to the new
     /// location in memory.
+    /// This function can return a `std::ptr::null_mut()` to signal an error.
     fn remap(&self, ptr: *mut u8, oldsize: usize, newsize: usize, can_move: bool) -> *mut u8;
 
-    /// Resizes a memory chunk starting at `ptr` with size `oldsize` to a memory region of
-    /// `newsize` bytes. Returns `true` if the memory region could be resized.
+    /// Frees a part of a memory chunk. The original memory chunk starts at `ptr` with size `oldsize`
+    /// and is turned into a memory region starting at the same address but with `newsize` bytes.
+    /// Returns `true` iff the access memory region could be freed.
     fn free_part(&self, ptr: *mut u8, oldsize: usize, newsize: usize) -> bool;
 
-    /// Frees an entire memory region. Returns `true` if the operation succeeded
+    /// Frees an entire memory region. Returns `true` iff the operation succeeded. When `false` is
+    /// returned, the `dlmalloc` may re-use the location on future allocation requests
     fn free(&self, ptr: *mut u8, size: usize) -> bool;
 
-    /// Indicates if the platform can release a part of memory. For the `flags` argument, see
-    /// `System::alloc`
+    /// Indicates if the system can release a part of memory. For the `flags` argument, see
+    /// `Allocator::alloc`
     fn can_release_part(&self, flags: u32) -> bool;
 
     /// Indicates whether newly allocated regions contain zeros.
     fn allocates_zeros(&self) -> bool;
 
-    /// Returns the page size
+    /// Returns the page size. Must be a power of two
     fn page_size(&self) -> usize;
 }
 
@@ -68,16 +72,7 @@ pub unsafe trait System: Send {
 /// Instances of this type are used to allocate blocks of memory. For best
 /// results only use one of these. Currently doesn't implement `Drop` to release
 /// lingering memory back to the OS. That may happen eventually though!
-#[cfg(any(target_os = "linux", target_arch = "wasm32", target_os = "macos"))]
-pub struct Dlmalloc<S = Platform>(dlmalloc::Dlmalloc<S>);
-
-/// An allocator instance
-///
-/// Instances of this type are used to allocate blocks of memory. For best
-/// results only use one of these. Currently doesn't implement `Drop` to release
-/// lingering memory back to the OS. That may happen eventually though!
-#[cfg(not(any(target_os = "linux", target_arch = "wasm32", target_os = "macos")))]
-pub struct Dlmalloc<S>(dlmalloc::Dlmalloc<S>);
+pub struct Dlmalloc<S = System>(dlmalloc::Dlmalloc<S>);
 
 #[cfg(target_arch = "wasm32")]
 #[path = "wasm.rs"]
@@ -92,63 +87,24 @@ mod sys;
 mod sys;
 
 #[cfg(not(any(target_os = "linux", target_os = "macos", target_arch = "wasm32")))]
-pub struct Platform {
-    _priv: (),
-}
-
-#[cfg(not(any(target_os = "linux", target_os = "macos", target_arch = "wasm32")))]
-impl Platform {
-    const fn new() -> Platform {
-        Platform { _priv: () }
-    }
-}
-
-#[cfg(not(any(target_os = "linux", target_os = "macos", target_arch = "wasm32")))]
-unsafe impl System for Platform {
-    fn alloc(&self, size: usize) -> (*mut u8, usize, u32) {
-        panic!("Please implement the `System` interface and use the `Dlmalloc::new_with_platform` method");
-    }
-
-    fn remap(&self, ptr: *mut u8, oldsize: usize, newsize: usize, can_move: bool) -> *mut u8 {
-        panic!("Please implement the `System` interface and use the `Dlmalloc::new_with_platform` method");
-    }
-
-    fn free_part(&self, ptr: *mut u8, oldsize: usize, newsize: usize) -> bool {
-        panic!("Please implement the `System` interface and use the `Dlmalloc::new_with_platform` method");
-    }
-
-    fn free(&self, ptr: *mut u8, size: usize) -> bool {
-        panic!("Please implement the `System` interface and use the `Dlmalloc::new_with_platform` method");
-    }
-
-    fn can_release_part(&self, flags: u32) -> bool {
-        panic!("Please implement the `System` interface and use the `Dlmalloc::new_with_platform` method");
-    }
-
-    fn allocates_zeros(&self) -> bool {
-        panic!("Please implement the `System` interface and use the `Dlmalloc::new_with_platform` method");
-    }
-
-    fn page_size(&self) -> usize {
-        panic!("Please implement the `System` interface and use the `Dlmalloc::new_with_platform` method");
-    }
-}
+#[path = "dummy.rs"]
+mod sys;
 
-impl Dlmalloc<Platform> {
+impl Dlmalloc<System> {
     /// Creates a new instance of an allocator
-    pub const fn new() -> Dlmalloc<Platform> {
-        Dlmalloc(dlmalloc::Dlmalloc::new(Platform::new()))
+    pub const fn new() -> Dlmalloc<System> {
+        Dlmalloc(dlmalloc::Dlmalloc::new(System::new()))
     }
 }
 
 impl<S> Dlmalloc<S> {
     /// Creates a new instance of an allocator
-    pub const fn new_with_platform(sys_allocator: S) -> Dlmalloc<S> {
+    pub const fn new_with_system(sys_allocator: S) -> Dlmalloc<S> {
         Dlmalloc(dlmalloc::Dlmalloc::new(sys_allocator))
     }
 }
 
-impl<S: System> Dlmalloc<S> {
+impl<S: Allocator> Dlmalloc<S> {
     /// Allocates `size` bytes with `align` align.
     ///
     /// Returns a null pointer if allocation fails. Returns a valid pointer

src/linux.rs

@@ -1,23 +1,23 @@
 extern crate libc;
 
 use core::ptr;
-use System;
+use Allocator;
 
 /// System setting for Linux
-pub struct Platform {
+pub struct System {
     _priv: (),
 }
 
-impl Platform {
-    pub const fn new() -> Platform {
-        Platform { _priv: () }
+impl System {
+    pub const fn new() -> System {
+        System { _priv: () }
     }
 }
 
 #[cfg(feature = "global")]
 static mut LOCK: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
 
-unsafe impl System for Platform {
+unsafe impl Allocator for System {
     fn alloc(&self, size: usize) -> (*mut u8, usize, u32) {
         let addr = unsafe {
             libc::mmap(

src/macos.rs

@@ -1,23 +1,23 @@
 extern crate libc;
 
 use core::ptr;
-use System;
+use Allocator;
 
 /// System setting for MacOS
-pub struct Platform {
+pub struct System {
     _priv: (),
 }
 
-impl Platform {
-    pub const fn new() -> Platform {
-        Platform { _priv: () }
+impl System {
+    pub const fn new() -> System {
+        System { _priv: () }
     }
 }
 
 #[cfg(feature = "global")]
 static mut LOCK: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
 
-unsafe impl System for Platform {
+unsafe impl Allocator for System {
     fn alloc(&self, size: usize) -> (*mut u8, usize, u32) {
         let addr = unsafe {
             libc::mmap(

src/wasm.rs

@@ -1,19 +1,19 @@
 use core::arch::wasm32;
 use core::ptr;
-use System;
+use Allocator;
 
 /// System setting for Wasm
-pub struct Platform {
+pub struct System {
     _priv: (),
 }
 
-impl Platform {
-    pub const fn new() -> Platform {
-        Platform { _priv: () }
+impl System {
+    pub const fn new() -> System {
+        System { _priv: () }
     }
 }
 
-unsafe impl System for Platform {
+unsafe impl Allocator for System {
     fn alloc(&self, size: usize) -> (*mut u8, usize, u32) {
         let pages = size / self.page_size();
         let prev = wasm32::memory_grow(0, pages);