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.

Sign up for GitHub

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

Jump to bottom

server: Don't swallow errors in register_device_collection_changed. #71

Merged
merged 2 commits into from Jul 29, 2019
Merged

server: Don't swallow errors in register_device_collection_changed. #71

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
097c9ef
server: Don't swallow errors in register_device_collection_changed.
kinetiknz Jul 29, 2019
65e0be1
Refactor CubebDeviceCollectionManager::update slightly.
kinetiknz Jul 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 26 additions & 18 deletions server/src/server.rs
View file
Open in desktop
Expand Up @@ -50,7 +50,11 @@ impl CubebDeviceCollectionManager {
}
}

fn register(&mut self, context: &cubeb::Context, server: &Rc<RefCell<CubebServerCallbacks>>) {
fn register(
&mut self,
context: &cubeb::Context,
server: &Rc<RefCell<CubebServerCallbacks>>,
) -> cubeb::Result<()> {
if self
.servers
.iter()
Expand All @@ -59,35 +63,40 @@ impl CubebDeviceCollectionManager {
{
self.servers.push(server.clone());
}
self.update(context);
self.update(context)
}

fn unregister(&mut self, context: &cubeb::Context, server: &Rc<RefCell<CubebServerCallbacks>>) {
fn unregister(
&mut self,
context: &cubeb::Context,
server: &Rc<RefCell<CubebServerCallbacks>>,
) -> cubeb::Result<()> {
self.servers
.retain(|s| !(Rc::ptr_eq(&s, server) && s.borrow().devtype.is_empty()));
self.update(context);
self.update(context)
}

fn update(&mut self, context: &cubeb::Context) {
fn update(&mut self, context: &cubeb::Context) -> cubeb::Result<()> {
let mut devtype = cubeb::DeviceType::empty();
for s in &self.servers {
devtype |= s.borrow().devtype;
}
for &dir in &[cubeb::DeviceType::INPUT, cubeb::DeviceType::OUTPUT] {
match (devtype.contains(dir), self.devtype.contains(dir)) {
(true, false) => self.internal_register(context, dir, true),
(false, true) => self.internal_register(context, dir, false),
(true, false) => self.internal_register(context, dir, true)?,
(false, true) => self.internal_register(context, dir, false)?,
Copy link
Member

Choose a reason for hiding this comment

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

for &dir in &[cubeb::DeviceType::INPUT, cubeb::DeviceType::OUTPUT] {
    if devtype.contains(dir) != self.devtype.contains(dir) {
        self.internal_register(context, dir, devtype.contains(dir))?;
    }
}

will be shorter

_ => {}
}
}
Ok(())
}

fn internal_register(
&mut self,
context: &cubeb::Context,
devtype: cubeb::DeviceType,
enable: bool,
) {
) -> cubeb::Result<()> {
let user_ptr = if enable {
self as *const CubebDeviceCollectionManager as *mut c_void
} else {
Expand All @@ -106,13 +115,11 @@ impl CubebDeviceCollectionManager {
if devtype.contains(dir) {
assert_eq!(self.devtype.contains(dir), !enable);
unsafe {
context
.register_device_collection_changed(
dir,
if enable { Some(cb) } else { None },
user_ptr,
)
.expect("devcol register failed");
context.register_device_collection_changed(
dir,
if enable { Some(cb) } else { None },
user_ptr,
)?;
}
if enable {
self.devtype.insert(dir);
Expand All @@ -121,6 +128,7 @@ impl CubebDeviceCollectionManager {
}
}
}
Ok(())
}

// Warning: this is called from an internal cubeb thread, so we must not mutate unprotected shared state.
Expand Down Expand Up @@ -540,12 +548,12 @@ impl CubebServer {

if enable {
cbs.borrow_mut().devtype.insert(devtype);
manager.register(context, cbs);
manager.register(context, cbs)
} else {
cbs.borrow_mut().devtype.remove(devtype);
manager.unregister(context, cbs);
manager.unregister(context, cbs)
}
Ok(ClientMessage::ContextRegisteredDeviceCollectionChanged)
.map(|_| ClientMessage::ContextRegisteredDeviceCollectionChanged)
}

// Stream init is special, so it's been separated from process_msg.
Expand Down