fix: factor out some unwraps in xstate::selection

Brought to my attention by #252, several `unwrap`s exist on code whose
failure does not justify the termination of the program, so I took some
time to replace them with proper error handling.

The request for TARGETS in `handle_selection_request` used `unwrap`.
Since the code for a failure of responsing to `SelectionRequest` exists,
it simply uses that.

`handle_target_list` now early returns if receiving the
`GetProperty` request fails.

`handle_new_owner` does not update the timestamp for the last selection
on failure.

`set_owner` now returns a `bool` to indicate whether the
`SetSelectionOwner` and `GetSelectionOwner` requests succeeded and
whether they transferred ownership successfully or not. The call spots
have also been modified to not change the `selection_state` if this call
fails.
This commit is contained in:
En-En 2025-10-19 13:35:58 +00:00 committed by Supreeeme
parent d621a0b37a
commit 04816e2a36

View file

@ -238,7 +238,7 @@ struct SelectionData<T: SelectionType> {
// This is a trait so that we can use &dyn // This is a trait so that we can use &dyn
trait SelectionDataImpl { trait SelectionDataImpl {
fn set_owner(&self, connection: &xcb::Connection, wm_window: x::Window); fn set_owner(&self, connection: &xcb::Connection, wm_window: x::Window) -> bool;
fn handle_new_owner( fn handle_new_owner(
&mut self, &mut self,
connection: &xcb::Connection, connection: &xcb::Connection,
@ -288,27 +288,41 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
fn atom(&self) -> x::Atom { fn atom(&self) -> x::Atom {
self.atom self.atom
} }
fn set_owner(&self, connection: &xcb::Connection, wm_window: x::Window) { fn set_owner(&self, connection: &xcb::Connection, wm_window: x::Window) -> bool {
connection if let Err(e) = connection.send_and_check_request(&x::SetSelectionOwner {
.send_and_check_request(&x::SetSelectionOwner {
owner: wm_window, owner: wm_window,
selection: self.atom, selection: self.atom,
time: self.last_selection_timestamp, time: self.last_selection_timestamp,
}) }) {
.unwrap(); warn!(
"Could not become owner of {}: {e:?}",
get_atom_name(connection, self.atom)
);
return false;
};
let reply = connection match connection.wait_for_reply(connection.send_request(&x::GetSelectionOwner {
.wait_for_reply(connection.send_request(&x::GetSelectionOwner {
selection: self.atom, selection: self.atom,
})) })) {
.unwrap(); Ok(reply) => {
if reply.owner() != wm_window { if reply.owner() != wm_window {
warn!( warn!(
"Could not get {} selection (owned by {:?})", "Could not become owner of {} (owned by {:?})",
get_atom_name(connection, self.atom), get_atom_name(connection, self.atom),
reply.owner() reply.owner()
); );
false
} else {
true
}
}
Err(e) => {
warn!(
"Could not validate ownership of {}: {e:?}",
get_atom_name(connection, self.atom)
);
false
}
} }
} }
@ -320,21 +334,26 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
owner: x::Window, owner: x::Window,
timestamp: u32, timestamp: u32,
) { ) {
debug!(
"new {} owner: {owner:?}",
get_atom_name(connection, self.atom)
);
self.last_selection_timestamp = timestamp;
// Grab targets // Grab targets
connection match connection.send_and_check_request(&x::ConvertSelection {
.send_and_check_request(&x::ConvertSelection {
requestor: wm_window, requestor: wm_window,
selection: self.atom, selection: self.atom,
target: atoms.targets, target: atoms.targets,
property: atoms.selection_reply, property: atoms.selection_reply,
time: timestamp, time: timestamp,
}) }) {
.unwrap(); Ok(_) => {
debug!(
"new {} owner: {owner:?}",
get_atom_name(connection, self.atom)
);
self.last_selection_timestamp = timestamp;
}
Err(e) => warn!(
"could not set new {} owner: {e:?}",
get_atom_name(connection, self.atom)
),
}
} }
fn handle_target_list( fn handle_target_list(
@ -346,16 +365,20 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
dest_property: x::Atom, dest_property: x::Atom,
server_state: &mut RealServerState, server_state: &mut RealServerState,
) { ) {
let reply = connection let reply = match connection.wait_for_reply(connection.send_request(&x::GetProperty {
.wait_for_reply(connection.send_request(&x::GetProperty {
delete: true, delete: true,
window: wm_window, window: wm_window,
property: dest_property, property: dest_property,
r#type: x::ATOM_ATOM, r#type: x::ATOM_ATOM,
long_offset: 0, long_offset: 0,
long_length: 20, long_length: 20,
})) })) {
.unwrap(); Ok(reply) => reply,
Err(e) => {
warn!("Could not obtain target list: {e:?}");
return;
}
};
let targets: &[x::Atom] = reply.value(); let targets: &[x::Atom] = reply.value();
if targets.is_empty() { if targets.is_empty() {
@ -445,17 +468,19 @@ impl<T: SelectionType> SelectionDataImpl for SelectionData<T> {
if req_target == atoms.targets { if req_target == atoms.targets {
let atoms: Box<[x::Atom]> = mimes.iter().map(|t| t.atom).collect(); let atoms: Box<[x::Atom]> = mimes.iter().map(|t| t.atom).collect();
connection match connection.send_and_check_request(&x::ChangeProperty {
.send_and_check_request(&x::ChangeProperty {
mode: x::PropMode::Replace, mode: x::PropMode::Replace,
window: request.requestor(), window: request.requestor(),
property: request.property(), property: request.property(),
r#type: x::ATOM_ATOM, r#type: x::ATOM_ATOM,
data: &atoms, data: &atoms,
}) }) {
.unwrap(); Ok(_) => true,
Err(e) => {
true warn!("Failed to set targets for selection request: {e:?}");
false
}
}
} else { } else {
let Some(target) = mimes.iter().find(|t| t.atom == req_target) else { let Some(target) = mimes.iter().find(|t| t.atom == req_target) else {
if log::log_enabled!(log::Level::Debug) { if log::log_enabled!(log::Level::Debug) {
@ -603,17 +628,21 @@ impl XState {
}); });
} }
if self
.selection_state
.clipboard
.set_owner(&self.connection, self.wm_window)
{
self.selection_state.clipboard.current_selection = self.selection_state.clipboard.current_selection =
Some(CurrentSelection::Wayland(WaylandSelection { Some(CurrentSelection::Wayland(WaylandSelection {
mimes, mimes,
inner: selection, inner: selection,
incr_data: None, incr_data: None,
})); }));
self.selection_state
.clipboard
.set_owner(&self.connection, self.wm_window);
debug!("Clipboard set from Wayland"); debug!("Clipboard set from Wayland");
} }
}
pub(crate) fn set_primary_selection(&mut self, selection: ForeignSelection<Primary>) { pub(crate) fn set_primary_selection(&mut self, selection: ForeignSelection<Primary>) {
let mut utf8_xwl = false; let mut utf8_xwl = false;
@ -661,17 +690,20 @@ impl XState {
}); });
} }
if self
.selection_state
.primary
.set_owner(&self.connection, self.wm_window)
{
self.selection_state.primary.current_selection = self.selection_state.primary.current_selection =
Some(CurrentSelection::Wayland(WaylandSelection { Some(CurrentSelection::Wayland(WaylandSelection {
mimes, mimes,
inner: selection, inner: selection,
incr_data: None, incr_data: None,
})); }));
self.selection_state
.primary
.set_owner(&self.connection, self.wm_window);
debug!("Primary set from Wayland"); debug!("Primary set from Wayland");
} }
}
pub(super) fn handle_selection_event( pub(super) fn handle_selection_event(
&mut self, &mut self,