diwic / alsa-rs

Thin but safe ALSA wrappers for Rust
Apache License 2.0
139 stars 66 forks source link

Setter functions not requiring mutable references #69

Closed xTibor closed 3 years ago

xTibor commented 3 years ago

Consider the following example:

use alsa::seq::{Addr, PortSubscribe};

fn foo(sub: &PortSubscribe) {
    sub.set_dest(Addr {
        client: 123,
        port: 45,
    });
}

fn main() {
    let sub = PortSubscribe::empty().unwrap();
    println!("{:?}", sub.get_dest());
    foo(&sub);
    println!("{:?}", sub.get_dest());
}

This just feels unsound to me, mutating the state of ALSA types without requiring mutable references. PortSubscribe is not the only one affected by this, there are dozens of setter functions in this crate without proper mutability constraints.

diwic commented 3 years ago

Well, we're interfacing with C, and C works differently w r t mutability. If it helps, you can think of every C struct as being internally constructed by fields which are all Cell (or some variation thereof). Just like Cell allows for mutation behind a & reference without being unsound, these structs do too.

This is also why PortSubscribe and friends are not Sync, only Send. Can you find some place where a struct that is Sync allows for mutation with only a & reference, let me know, and I'll have an extra look at that.

diwic commented 3 years ago

I'm closing this for now, but please reopen if you feel that my reasoning is wrong or you can find some actual unsoundness.