mirror
/
edit
mirror of https://github.com/microsoft/edit
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Use NonNull to represent valid sys handles

This commit is contained in:
Leonard Hecker 2025-04-13 01:52:52 +02:00
parent 4cf25ae025
commit ee5b5a0bfa
4 changed files with 57 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
src/buffer.rs
View File

@ -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 = v.as_mut_ptr();
buffer = BackingBuffer::Vec(v);
text = NonNull::dangling();
buffer = BackingBuffer::Vec(Vec::new());
} 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,15 +2417,18 @@ impl GapBuffer {
let gap_beg = unsafe { self.text.add(off) };
unsafe {
ptr::copy(
gap_beg.add(gap_len_old),
gap_beg.add(gap_len_new),
gap_beg.add(gap_len_old).as_ptr(),
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(
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) }
}
}

2
src/memchr.rs
View File

@ -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');

38
src/sys/unix.rs
View File

@ -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::MaybeUninit;
use std::mem::{self, MaybeUninit};
use std::os::fd::FromRawFd;
use std::ptr;
use std::ptr::{null, null_mut};
use std::ptr::{self, NonNull, 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);
if handle.is_null() {
Err(errno_to_apperr(libc::ELIBACC))
} else {
Ok(handle)
}
NonNull::new(libc::dlopen(name.as_ptr(), libc::RTLD_LAZY))
.ok_or_else(|| errno_to_apperr(libc::ELIBACC))
}
}
@ -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") }
}

34
src/sys/windows.rs
View File

@ -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> {
if ret.is_null() {
Err(get_last_error())
} else {
Ok(ret)
}
fn check_ptr_return<T>(ret: *mut T) -> apperr::Result<NonNull<T>> {
NonNull::new(ret).ok_or_else(get_last_error)
}