Skip to content

Commit 703ff60

Browse files
committed
Use NonNull in merge_sort
This is more clear about the intent of the pointer and avoids problems if the allocation returns a null pointer.
1 parent e098eb1 commit 703ff60

File tree

1 file changed

+19
-15
lines changed

1 file changed

+19
-15
lines changed

library/core/src/slice/sort.rs

+19-15
Original file line numberDiff line numberDiff line change
@@ -1203,7 +1203,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
12031203
// `is_less` panics. When merging two sorted runs, this buffer holds a copy of the shorter run,
12041204
// which will always have length at most `len / 2`.
12051205
let buf = BufGuard::new(len / 2, elem_alloc_fn, elem_dealloc_fn);
1206-
let buf_ptr = buf.buf_ptr;
1206+
let buf_ptr = buf.buf_ptr.as_ptr();
12071207

12081208
let mut runs = RunVec::new(run_alloc_fn, run_dealloc_fn);
12091209

@@ -1298,7 +1298,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
12981298
where
12991299
ElemDeallocF: Fn(*mut T, usize),
13001300
{
1301-
buf_ptr: *mut T,
1301+
buf_ptr: ptr::NonNull<T>,
13021302
capacity: usize,
13031303
elem_dealloc_fn: ElemDeallocF,
13041304
}
@@ -1315,7 +1315,11 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
13151315
where
13161316
ElemAllocF: Fn(usize) -> *mut T,
13171317
{
1318-
Self { buf_ptr: elem_alloc_fn(len), capacity: len, elem_dealloc_fn }
1318+
Self {
1319+
buf_ptr: ptr::NonNull::new(elem_alloc_fn(len)).unwrap(),
1320+
capacity: len,
1321+
elem_dealloc_fn,
1322+
}
13191323
}
13201324
}
13211325

@@ -1324,7 +1328,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
13241328
ElemDeallocF: Fn(*mut T, usize),
13251329
{
13261330
fn drop(&mut self) {
1327-
(self.elem_dealloc_fn)(self.buf_ptr, self.capacity);
1331+
(self.elem_dealloc_fn)(self.buf_ptr.as_ptr(), self.capacity);
13281332
}
13291333
}
13301334

@@ -1333,7 +1337,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
13331337
RunAllocF: Fn(usize) -> *mut TimSortRun,
13341338
RunDeallocF: Fn(*mut TimSortRun, usize),
13351339
{
1336-
buf_ptr: *mut TimSortRun,
1340+
buf_ptr: ptr::NonNull<TimSortRun>,
13371341
capacity: usize,
13381342
len: usize,
13391343
run_alloc_fn: RunAllocF,
@@ -1350,7 +1354,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
13501354
const START_RUN_CAPACITY: usize = 16;
13511355

13521356
Self {
1353-
buf_ptr: run_alloc_fn(START_RUN_CAPACITY),
1357+
buf_ptr: ptr::NonNull::new(run_alloc_fn(START_RUN_CAPACITY)).unwrap(),
13541358
capacity: START_RUN_CAPACITY,
13551359
len: 0,
13561360
run_alloc_fn,
@@ -1361,23 +1365,23 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
13611365
fn push(&mut self, val: TimSortRun) {
13621366
if self.len == self.capacity {
13631367
let old_capacity = self.capacity;
1364-
let old_buf_ptr = self.buf_ptr;
1368+
let old_buf_ptr = self.buf_ptr.as_ptr();
13651369

13661370
self.capacity = self.capacity * 2;
1367-
self.buf_ptr = (self.run_alloc_fn)(self.capacity);
1371+
self.buf_ptr = ptr::NonNull::new((self.run_alloc_fn)(self.capacity)).unwrap();
13681372

13691373
// SAFETY: buf_ptr new and old were correctly allocated and old_buf_ptr has
13701374
// old_capacity valid elements.
13711375
unsafe {
1372-
ptr::copy_nonoverlapping(old_buf_ptr, self.buf_ptr, old_capacity);
1376+
ptr::copy_nonoverlapping(old_buf_ptr, self.buf_ptr.as_ptr(), old_capacity);
13731377
}
13741378

13751379
(self.run_dealloc_fn)(old_buf_ptr, old_capacity);
13761380
}
13771381

13781382
// SAFETY: The invariant was just checked.
13791383
unsafe {
1380-
self.buf_ptr.add(self.len).write(val);
1384+
self.buf_ptr.as_ptr().add(self.len).write(val);
13811385
}
13821386
self.len += 1;
13831387
}
@@ -1390,7 +1394,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
13901394
// SAFETY: buf_ptr needs to be valid and len invariant upheld.
13911395
unsafe {
13921396
// the place we are taking from.
1393-
let ptr = self.buf_ptr.add(index);
1397+
let ptr = self.buf_ptr.as_ptr().add(index);
13941398

13951399
// Shift everything down to fill in that spot.
13961400
ptr::copy(ptr.add(1), ptr, self.len - index - 1);
@@ -1400,7 +1404,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
14001404

14011405
fn as_slice(&self) -> &[TimSortRun] {
14021406
// SAFETY: Safe as long as buf_ptr is valid and len invariant was upheld.
1403-
unsafe { &*ptr::slice_from_raw_parts(self.buf_ptr, self.len) }
1407+
unsafe { &*ptr::slice_from_raw_parts(self.buf_ptr.as_ptr(), self.len) }
14041408
}
14051409

14061410
fn len(&self) -> usize {
@@ -1419,7 +1423,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
14191423
if index < self.len {
14201424
// SAFETY: buf_ptr and len invariant must be upheld.
14211425
unsafe {
1422-
return &*(self.buf_ptr.add(index));
1426+
return &*(self.buf_ptr.as_ptr().add(index));
14231427
}
14241428
}
14251429

@@ -1436,7 +1440,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
14361440
if index < self.len {
14371441
// SAFETY: buf_ptr and len invariant must be upheld.
14381442
unsafe {
1439-
return &mut *(self.buf_ptr.add(index));
1443+
return &mut *(self.buf_ptr.as_ptr().add(index));
14401444
}
14411445
}
14421446

@@ -1452,7 +1456,7 @@ pub fn merge_sort<T, CmpF, ElemAllocF, ElemDeallocF, RunAllocF, RunDeallocF>(
14521456
fn drop(&mut self) {
14531457
// As long as TimSortRun is Copy we don't need to drop them individually but just the
14541458
// whole allocation.
1455-
(self.run_dealloc_fn)(self.buf_ptr, self.capacity);
1459+
(self.run_dealloc_fn)(self.buf_ptr.as_ptr(), self.capacity);
14561460
}
14571461
}
14581462
}

0 commit comments

Comments
 (0)