Skip to content

Commit

Permalink
saner parameter order for reallocation functions
Browse files Browse the repository at this point in the history
Using reallocate(old_ptr, old_size, new_size, align) makes a lot more
sense than reallocate(old_ptr, new_size, align, old_size) and matches up
with the order used by existing platform APIs like mremap.

Closes #17837

[breaking-change]
  • Loading branch information
thestinger committed Oct 8, 2014
1 parent 593174b commit 1c6fd76
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 43 deletions.
33 changes: 14 additions & 19 deletions src/liballoc/heap.rs
Expand Up @@ -31,9 +31,8 @@ pub unsafe fn allocate(size: uint, align: uint) -> *mut u8 {
/// create the allocation referenced by `ptr`. The `old_size` parameter may also
/// be the value returned by `usable_size` for the requested size.
#[inline]
pub unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint,
old_size: uint) -> *mut u8 {
imp::reallocate(ptr, size, align, old_size)
pub unsafe fn reallocate(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> *mut u8 {
imp::reallocate(ptr, old_size, size, align)
}

/// Extends or shrinks the allocation referenced by `ptr` to `size` bytes of
Expand All @@ -50,9 +49,8 @@ pub unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint,
/// create the allocation referenced by `ptr`. The `old_size` parameter may be
/// any value in range_inclusive(requested_size, usable_size).
#[inline]
pub unsafe fn reallocate_inplace(ptr: *mut u8, size: uint, align: uint,
old_size: uint) -> bool {
imp::reallocate_inplace(ptr, size, align, old_size)
pub unsafe fn reallocate_inplace(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> bool {
imp::reallocate_inplace(ptr, old_size, size, align)
}

/// Deallocates the memory referenced by `ptr`.
Expand Down Expand Up @@ -170,8 +168,7 @@ mod imp {
}

#[inline]
pub unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint,
_old_size: uint) -> *mut u8 {
pub unsafe fn reallocate(ptr: *mut u8, _old_size: uint, size: uint, align: uint) -> *mut u8 {
let flags = align_to_flags(align);
let ptr = je_rallocx(ptr as *mut c_void, size as size_t, flags) as *mut u8;
if ptr.is_null() {
Expand All @@ -181,8 +178,8 @@ mod imp {
}

#[inline]
pub unsafe fn reallocate_inplace(ptr: *mut u8, size: uint, align: uint,
old_size: uint) -> bool {
pub unsafe fn reallocate_inplace(ptr: *mut u8, old_size: uint, size: uint,
align: uint) -> bool {
let flags = align_to_flags(align);
let new_size = je_xallocx(ptr as *mut c_void, size as size_t, 0, flags) as uint;
// checking for failure to shrink is tricky
Expand Down Expand Up @@ -243,8 +240,7 @@ mod imp {
}

#[inline]
pub unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint,
old_size: uint) -> *mut u8 {
pub unsafe fn reallocate(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> *mut u8 {
if align <= MIN_ALIGN {
libc_heap::realloc_raw(ptr, size)
} else {
Expand All @@ -256,8 +252,8 @@ mod imp {
}

#[inline]
pub unsafe fn reallocate_inplace(_ptr: *mut u8, size: uint, _align: uint,
old_size: uint) -> bool {
pub unsafe fn reallocate_inplace(_ptr: *mut u8, old_size: uint, size: uint,
_align: uint) -> bool {
size == old_size
}

Expand Down Expand Up @@ -303,8 +299,7 @@ mod imp {
}

#[inline]
pub unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint,
_old_size: uint) -> *mut u8 {
pub unsafe fn reallocate(ptr: *mut u8, _old_size: uint, size: uint, align: uint) -> *mut u8 {
if align <= MIN_ALIGN {
libc_heap::realloc_raw(ptr, size)
} else {
Expand All @@ -318,8 +313,8 @@ mod imp {
}

#[inline]
pub unsafe fn reallocate_inplace(_ptr: *mut u8, size: uint, _align: uint,
old_size: uint) -> bool {
pub unsafe fn reallocate_inplace(_ptr: *mut u8, old_size: uint, size: uint,
_align: uint) -> bool {
size == old_size
}

Expand Down Expand Up @@ -351,7 +346,7 @@ mod test {
unsafe {
let size = 4000;
let ptr = heap::allocate(size, 8);
let ret = heap::reallocate_inplace(ptr, size, 8, size);
let ret = heap::reallocate_inplace(ptr, size, size, 8);
heap::deallocate(ptr, size, 8);
assert!(ret);
}
Expand Down
15 changes: 6 additions & 9 deletions src/libcollections/vec.rs
Expand Up @@ -622,12 +622,11 @@ impl<T: Clone> CloneableVector<T> for Vec<T> {

// FIXME: #13996: need a way to mark the return value as `noalias`
#[inline(never)]
unsafe fn alloc_or_realloc<T>(ptr: *mut T, size: uint, old_size: uint) -> *mut T {
unsafe fn alloc_or_realloc<T>(ptr: *mut T, old_size: uint, size: uint) -> *mut T {
if old_size == 0 {
allocate(size, mem::min_align_of::<T>()) as *mut T
} else {
reallocate(ptr as *mut u8, size,
mem::min_align_of::<T>(), old_size) as *mut T
reallocate(ptr as *mut u8, old_size, size, mem::min_align_of::<T>()) as *mut T
}
}

Expand Down Expand Up @@ -720,8 +719,7 @@ impl<T> Vec<T> {
let size = capacity.checked_mul(&mem::size_of::<T>())
.expect("capacity overflow");
unsafe {
self.ptr = alloc_or_realloc(self.ptr, size,
self.cap * mem::size_of::<T>());
self.ptr = alloc_or_realloc(self.ptr, self.cap * mem::size_of::<T>(), size);
}
self.cap = capacity;
}
Expand Down Expand Up @@ -751,9 +749,9 @@ impl<T> Vec<T> {
// Overflow check is unnecessary as the vector is already at
// least this large.
self.ptr = reallocate(self.ptr as *mut u8,
self.cap * mem::size_of::<T>(),
self.len * mem::size_of::<T>(),
mem::min_align_of::<T>(),
self.cap * mem::size_of::<T>()) as *mut T;
mem::min_align_of::<T>()) as *mut T;
}
self.cap = self.len;
}
Expand Down Expand Up @@ -1736,8 +1734,7 @@ impl<T> MutableSeq<T> for Vec<T> {
let size = max(old_size, 2 * mem::size_of::<T>()) * 2;
if old_size > size { fail!("capacity overflow") }
unsafe {
self.ptr = alloc_or_realloc(self.ptr, size,
self.cap * mem::size_of::<T>());
self.ptr = alloc_or_realloc(self.ptr, self.cap * mem::size_of::<T>(), size);
}
self.cap = max(self.cap, 2) * 2;
}
Expand Down
29 changes: 14 additions & 15 deletions src/test/run-pass/realloc-16687.rs
Expand Up @@ -63,19 +63,18 @@ unsafe fn test_triangle() -> bool {

heap::deallocate(ptr, size, align);
}
unsafe fn reallocate(ptr: *mut u8, size: uint, align: uint,
old_size: uint) -> *mut u8 {
unsafe fn reallocate(ptr: *mut u8, old_size: uint, size: uint, align: uint) -> *mut u8 {
if PRINT {
println!("reallocate(ptr=0x{:010x} size={:u} align={:u} old_size={:u})",
ptr as uint, size, align, old_size);
println!("reallocate(ptr=0x{:010x} old_size={:u} size={:u} align={:u})",
ptr as uint, old_size, size, align);
}

let ret = heap::reallocate(ptr, size, align, old_size);
let ret = heap::reallocate(ptr, old_size, size, align);

if PRINT {
println!("reallocate(ptr=0x{:010x} size={:u} align={:u} old_size={:u}) \
println!("reallocate(ptr=0x{:010x} old_size={:u} size={:u} align={:u}) \
ret: 0x{:010x}",
ptr as uint, size, align, old_size, ret as uint);
ptr as uint, old_size, size, align, ret as uint);
}
ret
}
Expand Down Expand Up @@ -125,10 +124,10 @@ unsafe fn test_triangle() -> bool {
let (p0, p1, old_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i));
assert!(old_size < new_size);

ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size);
ascend[2*i] = reallocate(p0, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());

ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size);
ascend[2*i+1] = reallocate(p1, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());
}
}
Expand All @@ -140,10 +139,10 @@ unsafe fn test_triangle() -> bool {
let (p0, p1, new_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i));
assert!(new_size < old_size);

ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size);
ascend[2*i] = reallocate(p0, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());

ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size);
ascend[2*i+1] = reallocate(p1, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());
}
}
Expand All @@ -155,10 +154,10 @@ unsafe fn test_triangle() -> bool {
let (p0, p1, old_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i));
assert!(old_size < new_size);

ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size);
ascend[2*i+1] = reallocate(p1, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());

ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size);
ascend[2*i] = reallocate(p0, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());
}
}
Expand All @@ -170,10 +169,10 @@ unsafe fn test_triangle() -> bool {
let (p0, p1, new_size) = (ascend[2*i], ascend[2*i+1], idx_to_size(i));
assert!(new_size < old_size);

ascend[2*i+1] = reallocate(p1, new_size, ALIGN, old_size);
ascend[2*i+1] = reallocate(p1, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());

ascend[2*i] = reallocate(p0, new_size, ALIGN, old_size);
ascend[2*i] = reallocate(p0, old_size, new_size, ALIGN);
sanity_check(ascend.as_slice());
}
}
Expand Down

21 comments on commit 1c6fd76

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/alloc = 1c6fd76 into auto

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/alloc = 1c6fd76 merged ok, testing candidate = 800e904

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/alloc = 1c6fd76 into auto

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/alloc = 1c6fd76 merged ok, testing candidate = 41fde28

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/alloc = 1c6fd76 into auto

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/alloc = 1c6fd76 merged ok, testing candidate = 54966aef

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/alloc = 1c6fd76 into auto

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/alloc = 1c6fd76 merged ok, testing candidate = f010aa5

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging thestinger/rust/alloc = 1c6fd76 into auto

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thestinger/rust/alloc = 1c6fd76 merged ok, testing candidate = e6cfb56

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 1c6fd76 Oct 9, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = e6cfb56

Please sign in to comment.