Re: [PATCH v8 0/2] Add dma coherent allocator abstraction

From: Pyrex
Date: Mon Feb 10 2025 - 03:54:40 EST


I'm nervy about the Rust code here.

- read()'s comment says it takes a snapshot, but it doesn't

- read()'s name implies it does a read, but it doesn't. It returns a live, dangerous view

- into_parts()'s comment claims to decrement the refcount. One, it doesn't. Two, it probably shouldn't, if it's supposed to transfer ownership.

- write() shouldn't take an immutable receiver without unsafe

- write() is unsound if used with the slice from read()

- the mutation in write() breaks read() without contradicting its `Safety` requirements

- write() memcpys T, which isn't explicitly Copy

This doesn't have to be this unsafe. AsBytes + FromBytes implies Copy (or at least it _should_) -- so the view could be of Cell.