search

rust-osdev / volatile

Apache License 2.0
71 stars 19 forks source link

WriteOnly shouldn't implement Clone #11

Closed Freax13 closed 4 years ago

Freax13 commented 4 years ago

WriteOnly simply derives Clone so it calls Volatile::clone to clone its field, https://github.com/embed-rs/volatile/blob/a5a6d786995df7cf07ed8689f79805da10ba2429/src/lib.rs#L210-L211 but Volatile::clone uses Volatile::read internally which might not work since the value is write-only. https://github.com/embed-rs/volatile/blob/a5a6d786995df7cf07ed8689f79805da10ba2429/src/lib.rs#L144-L148

Removing Clone from WriteOnly is obviously a breaking change, so I'm not sure what to do about this.

Freax13 commented 4 years ago

the same logic probably also applies to Debug

phil-opp commented 4 years ago

Thanks for reporting! I would be happy to merge a pull request that fixes this. The breaking change isn't a problem since we can just release it as a new semver-incompatible version.

@oli-obk I'm thinking about moving this repo to the rust-osdev organization, would that be fine with you?

oli-obk commented 4 years ago

I'm thinking about moving this repo to the rust-osdev organization, would that be fine with you?

yes