Use NonNull to represent valid sys handles

src/buffer.rs

@@ -31,7 +31,7 @@ use std::io::Write as _;
 use std::mem::{self, MaybeUninit};
 use std::ops::{Deref, DerefMut, Range};
 use std::path::Path;
-use std::ptr;
+use std::ptr::{self, NonNull};
 use std::rc::Rc;
 use std::slice;
 use std::str;
@@ -2250,7 +2250,7 @@ impl TextBuffer {
 }
 
 enum BackingBuffer {
-    VirtualMemory(*mut u8, usize),
+    VirtualMemory(NonNull<u8>, usize),
     Vec(Vec<u8>),
 }
 
@@ -2270,7 +2270,7 @@ impl Drop for BackingBuffer {
 /// This variant is optimized for large buffers and uses virtual memory.
 pub struct GapBuffer {
     /// Pointer to the buffer.
-    text: *mut u8,
+    text: NonNull<u8>,
     /// Maximum size of the buffer, including gap.
     reserve: usize,
     /// Size of the buffer, including gap.
@@ -2296,9 +2296,8 @@ impl GapBuffer {
         let text;
 
         if small {
-            let mut v = Vec::new();
+            text = NonNull::dangling();
-            text = v.as_mut_ptr();
+            buffer = BackingBuffer::Vec(Vec::new());
-            buffer = BackingBuffer::Vec(v);
         } else {
             text = unsafe { sys::virtual_reserve(RESERVE)? };
             buffer = BackingBuffer::VirtualMemory(text, RESERVE);
@@ -2355,10 +2354,15 @@ impl GapBuffer {
                 let move_dst = if left { off + gap_len } else { gap_off };
                 let move_len = if left { gap_off - off } else { off - gap_off };
 
-                unsafe { ptr::copy(data.add(move_src), data.add(move_dst), move_len) };
+                unsafe {
+                    data.add(move_src)
+                        .copy_to_nonoverlapping(data.add(move_dst), move_len)
+                };
 
                 if cfg!(debug_assertions) {
-                    unsafe { slice::from_raw_parts_mut(data.add(off), gap_len).fill(0xCD) };
+                    unsafe {
+                        slice::from_raw_parts_mut(data.add(off).as_ptr(), gap_len).fill(0xCD)
+                    };
                 }
             }
 
@@ -2368,7 +2372,8 @@ impl GapBuffer {
         // Delete the text
         if cfg!(debug_assertions) {
             unsafe {
-                slice::from_raw_parts_mut(self.text.add(off + self.gap_len), delete).fill(0xCD)
+                slice::from_raw_parts_mut(self.text.add(off + self.gap_len).as_ptr(), delete)
+                    .fill(0xCD)
             };
         }
         self.gap_len += delete;
@@ -2402,7 +2407,7 @@ impl GapBuffer {
                     },
                     BackingBuffer::Vec(v) => {
                         v.resize(bytes_new, 0);
-                        self.text = v.as_mut_ptr();
+                        self.text = unsafe { NonNull::new_unchecked(v.as_mut_ptr()) };
                     }
                 }
 
@@ -2412,16 +2417,19 @@ impl GapBuffer {
             let gap_beg = unsafe { self.text.add(off) };
             unsafe {
                 ptr::copy(
-                    gap_beg.add(gap_len_old),
+                    gap_beg.add(gap_len_old).as_ptr(),
-                    gap_beg.add(gap_len_new),
+                    gap_beg.add(gap_len_new).as_ptr(),
                     self.text_length - off,
                 )
             };
 
             if cfg!(debug_assertions) {
                 unsafe {
-                    slice::from_raw_parts_mut(gap_beg.add(gap_len_old), gap_len_new - gap_len_old)
+                    slice::from_raw_parts_mut(
-                        .fill(0xCD)
+                        gap_beg.add(gap_len_old).as_ptr(),
+                        gap_len_new - gap_len_old,
+                    )
+                    .fill(0xCD)
                 };
             }
 
@@ -2429,7 +2437,7 @@ impl GapBuffer {
         }
 
         self.generation = self.generation.wrapping_add(1);
-        unsafe { slice::from_raw_parts_mut(self.text.add(off), len) }
+        unsafe { slice::from_raw_parts_mut(self.text.add(off).as_ptr(), len) }
     }
 
     fn commit_gap(&mut self, len: usize) {
@@ -2514,7 +2522,7 @@ impl Document for GapBuffer {
             len = self.text_length - off;
         }
 
-        unsafe { slice::from_raw_parts(self.text.add(beg), len) }
+        unsafe { slice::from_raw_parts(self.text.add(beg).as_ptr(), len) }
     }
 
     fn read_backward(&self, off: usize) -> &[u8] {
@@ -2534,7 +2542,7 @@ impl Document for GapBuffer {
             len = off - self.gap_off;
         }
 
-        unsafe { slice::from_raw_parts(self.text.add(beg), len) }
+        unsafe { slice::from_raw_parts(self.text.add(beg).as_ptr(), len) }
     }
 }
 

src/memchr.rs

@@ -474,7 +474,7 @@ mod tests {
             // 3 pages: uncommitted, committed, uncommitted
             let ptr = sys::virtual_reserve(page_size * 3).unwrap();
             sys::virtual_commit(ptr.add(page_size), page_size).unwrap();
-            slice::from_raw_parts_mut(ptr.add(page_size), page_size)
+            slice::from_raw_parts_mut(ptr.add(page_size).as_ptr(), page_size)
         };
 
         page.fill(b'a');

src/sys/unix.rs

@@ -3,11 +3,9 @@ use crate::helpers;
 use std::borrow::Cow;
 use std::ffi::{CStr, CString, c_int, c_void};
 use std::fs::{self, File};
-use std::mem;
+use std::mem::{self, MaybeUninit};
-use std::mem::MaybeUninit;
 use std::os::fd::FromRawFd;
-use std::ptr;
+use std::ptr::{self, NonNull, null, null_mut};
-use std::ptr::{null, null_mut};
 use std::thread;
 use std::time;
 
@@ -323,7 +321,7 @@ pub fn open_stdin_if_redirected() -> Option<File> {
 ///
 /// This function is unsafe because it uses raw pointers.
 /// Don't forget to release the memory when you're done with it or you'll leak it.
-pub unsafe fn virtual_reserve(size: usize) -> apperr::Result<*mut u8> {
+pub unsafe fn virtual_reserve(size: usize) -> apperr::Result<NonNull<u8>> {
     unsafe {
         let ptr = libc::mmap(
             null_mut(),
@@ -333,10 +331,10 @@ pub unsafe fn virtual_reserve(size: usize) -> apperr::Result<*mut u8> {
             -1,
             0,
         );
-        if ptr::eq(ptr, libc::MAP_FAILED) {
+        if ptr.is_null() || ptr::eq(ptr, libc::MAP_FAILED) {
             Err(errno_to_apperr(libc::ENOMEM))
         } else {
-            Ok(ptr as *mut u8)
+            Ok(NonNull::new_unchecked(ptr as *mut u8))
         }
     }
 }
@@ -347,9 +345,9 @@ pub unsafe fn virtual_reserve(size: usize) -> apperr::Result<*mut u8> {
 ///
 /// This function is unsafe because it uses raw pointers.
 /// Make sure to only pass pointers acquired from `virtual_reserve`.
-pub unsafe fn virtual_release(base: *mut u8, size: usize) {
+pub unsafe fn virtual_release(base: NonNull<u8>, size: usize) {
     unsafe {
-        libc::munmap(base as *mut libc::c_void, size);
+        libc::munmap(base.cast().as_ptr(), size);
     }
 }
 
@@ -360,10 +358,10 @@ pub unsafe fn virtual_release(base: *mut u8, size: usize) {
 /// This function is unsafe because it uses raw pointers.
 /// Make sure to only pass pointers acquired from `virtual_reserve`
 /// and to pass a size less than or equal to the size passed to `virtual_reserve`.
-pub unsafe fn virtual_commit(base: *mut u8, size: usize) -> apperr::Result<()> {
+pub unsafe fn virtual_commit(base: NonNull<u8>, size: usize) -> apperr::Result<()> {
     unsafe {
         let status = libc::mprotect(
-            base as *mut libc::c_void,
+            base.cast().as_ptr(),
             size,
             libc::PROT_READ | libc::PROT_WRITE,
         );
@@ -375,14 +373,10 @@ pub unsafe fn virtual_commit(base: *mut u8, size: usize) -> apperr::Result<()> {
     }
 }
 
-unsafe fn load_library(name: &CStr) -> apperr::Result<*mut c_void> {
+unsafe fn load_library(name: &CStr) -> apperr::Result<NonNull<c_void>> {
     unsafe {
-        let handle = libc::dlopen(name.as_ptr(), libc::RTLD_LAZY);
+        NonNull::new(libc::dlopen(name.as_ptr(), libc::RTLD_LAZY))
-        if handle.is_null() {
+            .ok_or_else(|| errno_to_apperr(libc::ELIBACC))
-            Err(errno_to_apperr(libc::ELIBACC))
-        } else {
-            Ok(handle)
-        }
     }
 }
 
@@ -394,9 +388,9 @@ unsafe fn load_library(name: &CStr) -> apperr::Result<*mut c_void> {
 /// of the function you're loading. No type checks whatsoever are performed.
 //
 // It'd be nice to constrain T to std::marker::FnPtr, but that's unstable.
-pub unsafe fn get_proc_address<T>(handle: *mut c_void, name: &CStr) -> apperr::Result<T> {
+pub unsafe fn get_proc_address<T>(handle: NonNull<c_void>, name: &CStr) -> apperr::Result<T> {
     unsafe {
-        let sym = libc::dlsym(handle, name.as_ptr());
+        let sym = libc::dlsym(handle.as_ptr(), name.as_ptr());
         if sym.is_null() {
             Err(errno_to_apperr(libc::ELIBACC))
         } else {
@@ -405,11 +399,11 @@ pub unsafe fn get_proc_address<T>(handle: *mut c_void, name: &CStr) -> apperr::R
     }
 }
 
-pub fn load_libicuuc() -> apperr::Result<*mut c_void> {
+pub fn load_libicuuc() -> apperr::Result<NonNull<c_void>> {
     unsafe { load_library(c"libicuuc.so") }
 }
 
-pub fn load_libicui18n() -> apperr::Result<*mut c_void> {
+pub fn load_libicui18n() -> apperr::Result<NonNull<c_void>> {
     unsafe { load_library(c"libicui18n.so") }
 }
 

src/sys/windows.rs

@@ -1,12 +1,12 @@
 use crate::helpers::{CoordType, Size};
 use crate::{apperr, helpers};
-use std::ffi::{CStr, OsString};
+use std::ffi::{CStr, OsString, c_void};
 use std::fmt::Write as _;
 use std::fs::{self, File};
 use std::mem::MaybeUninit;
 use std::os::windows::io::FromRawHandle;
 use std::path::{Path, PathBuf};
-use std::ptr::{self, null, null_mut};
+use std::ptr::{self, NonNull, null, null_mut};
 use std::{mem, time};
 use windows_sys::Win32::Foundation;
 use windows_sys::Win32::Globalization;
@@ -424,7 +424,7 @@ pub fn canonicalize<P: AsRef<Path>>(path: P) -> apperr::Result<PathBuf> {
 ///
 /// This function is unsafe because it uses raw pointers.
 /// Don't forget to release the memory when you're done with it or you'll leak it.
-pub unsafe fn virtual_reserve(size: usize) -> apperr::Result<*mut u8> {
+pub unsafe fn virtual_reserve(size: usize) -> apperr::Result<NonNull<u8>> {
     unsafe {
         let mut base = null_mut();
 
@@ -449,9 +449,9 @@ pub unsafe fn virtual_reserve(size: usize) -> apperr::Result<*mut u8> {
 ///
 /// This function is unsafe because it uses raw pointers.
 /// Make sure to only pass pointers acquired from `virtual_reserve`.
-pub unsafe fn virtual_release(base: *mut u8, size: usize) {
+pub unsafe fn virtual_release(base: NonNull<u8>, size: usize) {
     unsafe {
-        Memory::VirtualFree(base as *mut _, size, Memory::MEM_RELEASE);
+        Memory::VirtualFree(base.as_ptr() as *mut _, size, Memory::MEM_RELEASE);
     }
 }
 
@@ -462,10 +462,10 @@ pub unsafe fn virtual_release(base: *mut u8, size: usize) {
 /// This function is unsafe because it uses raw pointers.
 /// Make sure to only pass pointers acquired from `virtual_reserve`
 /// and to pass a size less than or equal to the size passed to `virtual_reserve`.
-pub unsafe fn virtual_commit(base: *mut u8, size: usize) -> apperr::Result<()> {
+pub unsafe fn virtual_commit(base: NonNull<u8>, size: usize) -> apperr::Result<()> {
     unsafe {
         check_ptr_return(Memory::VirtualAlloc(
-            base as *mut _,
+            base.as_ptr() as *mut _,
             size,
             Memory::MEM_COMMIT,
             Memory::PAGE_READWRITE,
@@ -474,11 +474,11 @@ pub unsafe fn virtual_commit(base: *mut u8, size: usize) -> apperr::Result<()> {
     }
 }
 
-unsafe fn get_module(name: *const u16) -> apperr::Result<Foundation::HMODULE> {
+unsafe fn get_module(name: *const u16) -> apperr::Result<NonNull<c_void>> {
     unsafe { check_ptr_return(LibraryLoader::GetModuleHandleW(name)) }
 }
 
-unsafe fn load_library(name: *const u16) -> apperr::Result<Foundation::HMODULE> {
+unsafe fn load_library(name: *const u16) -> apperr::Result<NonNull<c_void>> {
     unsafe {
         check_ptr_return(LibraryLoader::LoadLibraryExW(
             name,
@@ -496,9 +496,9 @@ unsafe fn load_library(name: *const u16) -> apperr::Result<Foundation::HMODULE>
 /// of the function you're loading. No type checks whatsoever are performed.
 //
 // It'd be nice to constrain T to std::marker::FnPtr, but that's unstable.
-pub unsafe fn get_proc_address<T>(handle: Foundation::HMODULE, name: &CStr) -> apperr::Result<T> {
+pub unsafe fn get_proc_address<T>(handle: NonNull<c_void>, name: &CStr) -> apperr::Result<T> {
     unsafe {
-        let ptr = LibraryLoader::GetProcAddress(handle, name.as_ptr() as *const u8);
+        let ptr = LibraryLoader::GetProcAddress(handle.as_ptr(), name.as_ptr() as *const u8);
         if let Some(ptr) = ptr {
             Ok(mem::transmute_copy(&ptr))
         } else {
@@ -507,11 +507,11 @@ pub unsafe fn get_proc_address<T>(handle: Foundation::HMODULE, name: &CStr) -> a
     }
 }
 
-pub fn load_libicuuc() -> apperr::Result<Foundation::HMODULE> {
+pub fn load_libicuuc() -> apperr::Result<NonNull<c_void>> {
     unsafe { load_library(w!("icuuc.dll")) }
 }
 
-pub fn load_libicui18n() -> apperr::Result<Foundation::HMODULE> {
+pub fn load_libicui18n() -> apperr::Result<NonNull<c_void>> {
     unsafe { load_library(w!("icuin.dll")) }
 }
 
@@ -621,10 +621,6 @@ fn check_bool_return(ret: Foundation::BOOL) -> apperr::Result<()> {
     }
 }
 
-fn check_ptr_return<T>(ret: *mut T) -> apperr::Result<*mut T> {
+fn check_ptr_return<T>(ret: *mut T) -> apperr::Result<NonNull<T>> {
-    if ret.is_null() {
+    NonNull::new(ret).ok_or_else(get_last_error)
-        Err(get_last_error())
-    } else {
-        Ok(ret)
-    }
 }