Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use Box::leak instead of forget #8

Closed
wants to merge 10 commits into from
37 changes: 18 additions & 19 deletions .github/workflows/lint_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,31 @@ name: Lint and Test
on: [push, pull_request]

jobs:
fmt:
name: fmt
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- run: rustup update
- run: rustup component add rustfmt
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
with:
components: rustfmt,clippy
- run: cargo fmt -- --check
- run: cargo clippy --all-targets

clippy:
name: clippy
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- run: rustup update
- run: rustup component add clippy
- run: cargo clippy --all --all-targets --all-features
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- run: cargo build
- run: cargo test --verbose --all
env:
RUST_BACKTRACE: 1

test:
miri-test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v1
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@nightly
with:
submodules: true
- run: rustup update
- run: cargo build --all-features
- run: cargo test --verbose --all --all-features
env:
RUST_BACKTRACE: 1
components: miri
- run: cargo +nightly miri test
19 changes: 12 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const OWN_FLAG: usize = !LEN_MASK;
impl<'a> MownStr<'a> {
pub const fn from_str(other: &'a str) -> MownStr<'a> {
assert!(other.len() <= LEN_MASK);
// NB: The only 'const' constuctor for NonNull is new_unchecked
// NB: The only 'const' constructor for NonNull is new_unchecked
// so we need an unsafe block.

// SAFETY: we need a *mut u8 for new_unchecked,
Expand Down Expand Up @@ -140,17 +140,15 @@ impl<'a> From<&'a str> for MownStr<'a> {
}

impl<'a> From<Box<str>> for MownStr<'a> {
fn from(mut other: Box<str>) -> MownStr<'a> {
fn from(other: Box<str>) -> MownStr<'a> {
let len = other.len();
assert!(len <= LEN_MASK);
let addr = other.as_mut_ptr();
let addr = Box::leak(other).as_mut_ptr();
let addr = unsafe {
// SAFETY: ptr can not be null,
NonNull::new_unchecked(addr)
};

std::mem::forget(other);

let xlen = len | OWN_FLAG;
let _phd = PhantomData;
MownStr { addr, xlen, _phd }
Expand Down Expand Up @@ -217,7 +215,7 @@ impl<'a> Eq for MownStr<'a> {}

impl<'a> PartialOrd for MownStr<'a> {
fn partial_cmp(&self, other: &MownStr<'a>) -> Option<std::cmp::Ordering> {
self.deref().partial_cmp(other.deref())
Some(self.cmp(other))
}
}

Expand Down Expand Up @@ -477,12 +475,16 @@ mod test {
#[test]
fn no_memory_leak() {
// performs several MownStr allocation in sequence,
// droping each one before allocating the next one
// dropping each one before allocating the next one
// (unless the v.pop() line below is commented out).
//
// If there is no memory leak,
// the increase in memory should be roughly 1 time the allocated size;
// otherwise, it should be roghly 10 times that size.
//
// NB: in miri, the value returned by get_rss_anon is fake,
// so no memory leak will ever be detected;
// but the test is still executed in miri to detect UB.

let m0 = get_rss_anon();
println!("memory = {} kB", m0);
Expand Down Expand Up @@ -515,6 +517,9 @@ mod test {
const CAP: usize = 100_000_000;

fn get_rss_anon() -> usize {
if cfg!(miri) {
return CAP; // return dummy value, as miri can not open files
}
let txt = fs::read_to_string("/proc/self/status").expect("read proc status");
let txt = txt.split("RssAnon:").nth(1).unwrap();
let txt = txt.split(" kB").next().unwrap();
Expand Down