Skip to content

Commit

Permalink
backend: Use Message<_, BorrowedFd> rather than RawFd
Browse files Browse the repository at this point in the history
This is a breaking change to `wayland-backend`, but not to
`wayland-client` or `wayland-server`. I believe that should be fine.

This avoids an unsafe conversion in the `rs` backend.
  • Loading branch information
ids1024 committed Jan 16, 2024
1 parent bbcafb7 commit a697934
Show file tree
Hide file tree
Showing 12 changed files with 56 additions and 52 deletions.
4 changes: 2 additions & 2 deletions wayland-backend/src/client_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{
any::Any,
fmt,
os::unix::io::{BorrowedFd, OwnedFd},
os::unix::{io::RawFd, net::UnixStream},
os::unix::net::UnixStream,
sync::Arc,
};

Expand Down Expand Up @@ -227,7 +227,7 @@ impl Backend {
/// is `wl_registry.bind`), the `child_spec` must be provided.
pub fn send_request(
&self,
msg: Message<ObjectId, RawFd>,
msg: Message<ObjectId, BorrowedFd>,
data: Option<Arc<dyn ObjectData>>,
child_spec: Option<(&'static Interface, u32)>,
) -> Result<ObjectId, InvalidId> {
Expand Down
4 changes: 2 additions & 2 deletions wayland-backend/src/rs/client_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
fmt,
os::unix::io::{BorrowedFd, OwnedFd},
os::unix::{
io::{AsRawFd, RawFd},
io::AsRawFd,
net::UnixStream,
},
sync::{Arc, Condvar, Mutex, MutexGuard, Weak},
Expand Down Expand Up @@ -303,7 +303,7 @@ impl InnerBackend {

pub fn send_request(
&self,
Message { sender_id: ObjectId { id }, opcode, args }: Message<ObjectId, RawFd>,
Message { sender_id: ObjectId { id }, opcode, args }: Message<ObjectId, BorrowedFd>,
data: Option<Arc<dyn ObjectData>>,
child_spec: Option<(&'static Interface, u32)>,
) -> Result<ObjectId, InvalidId> {
Expand Down
4 changes: 2 additions & 2 deletions wayland-backend/src/rs/server_impl/client.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{
ffi::CString,
os::unix::io::{AsFd, BorrowedFd, OwnedFd},
os::unix::{io::RawFd, net::UnixStream},
os::unix::net::UnixStream,
sync::Arc,
};

Expand Down Expand Up @@ -106,7 +106,7 @@ impl<D> Client<D> {

pub(crate) fn send_event(
&mut self,
Message { sender_id: object_id, opcode, args }: Message<ObjectId, RawFd>,
Message { sender_id: object_id, opcode, args }: Message<ObjectId, BorrowedFd>,
pending_destructors: Option<&mut Vec<super::handle::PendingDestructor<D>>>,
) -> Result<(), InvalidId> {
if self.killed {
Expand Down
10 changes: 5 additions & 5 deletions wayland-backend/src/rs/server_impl/handle.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{
ffi::CString,
os::unix::io::OwnedFd,
os::unix::{io::RawFd, net::UnixStream},
os::unix::io::{BorrowedFd, OwnedFd},
os::unix::net::UnixStream,
sync::{Arc, Mutex, Weak},
};

Expand Down Expand Up @@ -173,7 +173,7 @@ impl InnerHandle {
}
}

pub fn send_event(&self, msg: Message<ObjectId, RawFd>) -> Result<(), InvalidId> {
pub fn send_event(&self, msg: Message<ObjectId, BorrowedFd>) -> Result<(), InvalidId> {
self.state.lock().unwrap().send_event(msg)
}

Expand Down Expand Up @@ -292,7 +292,7 @@ pub(crate) trait ErasedState: downcast_rs::Downcast {
&self,
id: InnerObjectId,
) -> Result<Arc<dyn std::any::Any + Send + Sync>, InvalidId>;
fn send_event(&mut self, msg: Message<ObjectId, RawFd>) -> Result<(), InvalidId>;
fn send_event(&mut self, msg: Message<ObjectId, BorrowedFd>) -> Result<(), InvalidId>;
fn post_error(&mut self, object_id: InnerObjectId, error_code: u32, message: CString);
fn kill_client(&mut self, client_id: InnerClientId, reason: DisconnectReason);
fn global_info(&self, id: InnerGlobalId) -> Result<GlobalInfo, InvalidId>;
Expand Down Expand Up @@ -416,7 +416,7 @@ impl<D> ErasedState for State<D> {
.map(|arc| arc.into_any_arc())
}

fn send_event(&mut self, msg: Message<ObjectId, RawFd>) -> Result<(), InvalidId> {
fn send_event(&mut self, msg: Message<ObjectId, BorrowedFd>) -> Result<(), InvalidId> {
self.clients
.get_client_mut(msg.sender_id.id.client_id.clone())?
.send_event(msg, Some(&mut self.pending_destructors))
Expand Down
41 changes: 23 additions & 18 deletions wayland-backend/src/rs/socket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl BufferedSocket {
//
// if false is returned, it means there is not enough space
// in the buffer
fn attempt_write_message(&mut self, msg: &Message<u32, RawFd>) -> IoResult<bool> {
fn attempt_write_message(&mut self, msg: &Message<u32, BorrowedFd>) -> IoResult<bool> {
match write_to_buffers(msg, self.out_data.get_writable_storage(), &mut self.out_fds) {
Ok(bytes_out) => {
self.out_data.advance(bytes_out);
Expand All @@ -172,7 +172,7 @@ impl BufferedSocket {
///
/// If the message is too big to fit in the buffer, the error `Error::Sys(E2BIG)`
/// will be returned.
pub fn write_message(&mut self, msg: &Message<u32, RawFd>) -> IoResult<()> {
pub fn write_message(&mut self, msg: &Message<u32, BorrowedFd>) -> IoResult<()> {
if !self.attempt_write_message(msg)? {
// the attempt failed, there is not enough space in the buffer
// we need to flush it
Expand Down Expand Up @@ -320,8 +320,8 @@ mod tests {
use crate::protocol::{AllowNull, Argument, ArgumentType, Message};

use std::ffi::CString;
use std::io;
use std::os::unix::io::BorrowedFd;
use std::os::unix::prelude::IntoRawFd;

use smallvec::smallvec;

Expand All @@ -335,18 +335,18 @@ mod tests {
//
// if arguments contain FDs, check that the fd point to
// the same file, rather than are the same number.
fn assert_eq_msgs<Fd: AsRawFd + std::fmt::Debug>(
msg1: &Message<u32, Fd>,
msg2: &Message<u32, Fd>,
fn assert_eq_msgs<Fd1: AsFd + std::fmt::Debug, Fd2: AsFd + std::fmt::Debug>(
msg1: Message<u32, Fd1>,
msg2: Message<u32, Fd2>,
) {
let msg1 = msg1.map_fd(|fd| fd.as_fd().try_clone_to_owned().unwrap());
let msg2 = msg2.map_fd(|fd| fd.as_fd().try_clone_to_owned().unwrap());
assert_eq!(msg1.sender_id, msg2.sender_id);
assert_eq!(msg1.opcode, msg2.opcode);
assert_eq!(msg1.args.len(), msg2.args.len());
for (arg1, arg2) in msg1.args.iter().zip(msg2.args.iter()) {
if let (Argument::Fd(fd1), Argument::Fd(fd2)) = (arg1, arg2) {
let fd1 = unsafe { BorrowedFd::borrow_raw(fd1.as_raw_fd()) };
let fd2 = unsafe { BorrowedFd::borrow_raw(fd2.as_raw_fd()) };
assert!(same_file(fd1, fd2));
assert!(same_file(fd1.as_fd(), fd2.as_fd()));
} else {
assert_eq!(arg1, arg2);
}
Expand Down Expand Up @@ -399,17 +399,19 @@ mod tests {
})
.unwrap();

assert_eq_msgs(&msg.map_fd(|fd| fd.as_raw_fd()), &ret_msg.map_fd(IntoRawFd::into_raw_fd));
assert_eq_msgs(msg, ret_msg);
}

#[test]
fn write_read_cycle_fd() {
let stdin = io::stdin().lock();
let stdout = io::stdout().lock();
let msg = Message {
sender_id: 42,
opcode: 7,
args: smallvec![
Argument::Fd(1), // stdin
Argument::Fd(0), // stdout
Argument::Fd(stdin.as_fd()),
Argument::Fd(stdout.as_fd()),
],
};

Expand All @@ -434,11 +436,14 @@ mod tests {
}
})
.unwrap();
assert_eq_msgs(&msg.map_fd(|fd| fd.as_raw_fd()), &ret_msg.map_fd(IntoRawFd::into_raw_fd));
assert_eq_msgs(msg, ret_msg);
}

#[test]
fn write_read_cycle_multiple() {
let stdin = io::stderr().lock();
let stdout = io::stderr().lock();
let stderr = io::stderr().lock();
let messages = vec![
Message {
sender_id: 42,
Expand All @@ -452,16 +457,16 @@ mod tests {
sender_id: 42,
opcode: 1,
args: smallvec![
Argument::Fd(1), // stdin
Argument::Fd(0), // stdout
Argument::Fd(stdin.as_fd()),
Argument::Fd(stdout.as_fd()),
],
},
Message {
sender_id: 42,
opcode: 2,
args: smallvec![
Argument::Uint(3),
Argument::Fd(2), // stderr
Argument::Fd(stderr.as_fd()),
],
},
];
Expand Down Expand Up @@ -495,7 +500,7 @@ mod tests {
}
assert_eq!(recv_msgs.len(), 3);
for (msg1, msg2) in messages.into_iter().zip(recv_msgs.into_iter()) {
assert_eq_msgs(&msg1.map_fd(|fd| fd.as_raw_fd()), &msg2.map_fd(IntoRawFd::into_raw_fd));
assert_eq_msgs(msg1, msg2);
}
}

Expand Down Expand Up @@ -534,6 +539,6 @@ mod tests {
})
.unwrap();

assert_eq_msgs(&msg.map_fd(|fd| fd.as_raw_fd()), &ret_msg.map_fd(IntoRawFd::into_raw_fd));
assert_eq_msgs(msg, ret_msg);
}
}
11 changes: 4 additions & 7 deletions wayland-backend/src/rs/wire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use std::collections::VecDeque;
use std::convert::TryInto;
use std::ffi::CStr;
use std::os::unix::io::RawFd;
use std::os::unix::io::{BorrowedFd, OwnedFd};

use crate::protocol::{Argument, ArgumentType, Message};
Expand Down Expand Up @@ -71,7 +70,7 @@ impl std::fmt::Display for MessageParseError {
///
/// Any serialized Fd will be `dup()`-ed in the process
pub fn write_to_buffers(
msg: &Message<u32, RawFd>,
msg: &Message<u32, BorrowedFd>,
payload: &mut [u8],
fds: &mut Vec<OwnedFd>,
) -> Result<usize, MessageWriteError> {
Expand Down Expand Up @@ -126,9 +125,7 @@ pub fn write_to_buffers(
Argument::NewId(n) => write_buf(n, payload)?,
Argument::Array(ref a) => write_array_to_payload(a, payload)?,
Argument::Fd(fd) => {
let dup_fd = unsafe { BorrowedFd::borrow_raw(fd) }
.try_clone_to_owned()
.map_err(MessageWriteError::DupFdFailed)?;
let dup_fd = fd.try_clone_to_owned().map_err(MessageWriteError::DupFdFailed)?;
fds.push(dup_fd);
payload
}
Expand Down Expand Up @@ -255,7 +252,7 @@ mod tests {
use super::*;
use crate::protocol::AllowNull;
use smallvec::smallvec;
use std::{ffi::CString, os::unix::io::IntoRawFd};
use std::ffi::CString;

#[test]
fn into_from_raw_cycle() {
Expand Down Expand Up @@ -293,6 +290,6 @@ mod tests {
&mut fd_buffer,
)
.unwrap();
assert_eq!(rebuilt.map_fd(IntoRawFd::into_raw_fd), msg);
assert_eq!(rebuilt, msg.map_fd(|fd| fd.try_clone_to_owned().unwrap()));
}
}
4 changes: 2 additions & 2 deletions wayland-backend/src/server_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{
ffi::CString,
fmt,
os::unix::io::{BorrowedFd, OwnedFd},
os::unix::{io::RawFd, net::UnixStream},
os::unix::net::UnixStream,
sync::Arc,
};

Expand Down Expand Up @@ -367,7 +367,7 @@ impl Handle {
/// - the message opcode must be valid for the sender interface
/// - the argument list must match the prototype for the message associated with this opcode
#[inline]
pub fn send_event(&self, msg: Message<ObjectId, RawFd>) -> Result<(), InvalidId> {
pub fn send_event(&self, msg: Message<ObjectId, BorrowedFd>) -> Result<(), InvalidId> {
self.handle.send_event(msg)
}

Expand Down
6 changes: 3 additions & 3 deletions wayland-backend/src/sys/client_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
os::raw::{c_int, c_void},
os::unix::io::{BorrowedFd, OwnedFd},
os::unix::{
io::{FromRawFd, IntoRawFd, RawFd},
io::{AsRawFd, FromRawFd, IntoRawFd},
net::UnixStream,
},
sync::{
Expand Down Expand Up @@ -520,7 +520,7 @@ impl InnerBackend {

pub fn send_request(
&self,
Message { sender_id: ObjectId { id }, opcode, args }: Message<ObjectId, RawFd>,
Message { sender_id: ObjectId { id }, opcode, args }: Message<ObjectId, BorrowedFd>,
data: Option<Arc<dyn ObjectData>>,
child_spec: Option<(&'static Interface, u32)>,
) -> Result<ObjectId, InvalidId> {
Expand Down Expand Up @@ -615,7 +615,7 @@ impl InnerBackend {
Argument::Uint(u) => argument_list.push(wl_argument { u }),
Argument::Int(i) => argument_list.push(wl_argument { i }),
Argument::Fixed(f) => argument_list.push(wl_argument { f }),
Argument::Fd(h) => argument_list.push(wl_argument { h }),
Argument::Fd(h) => argument_list.push(wl_argument { h: h.as_raw_fd() }),
Argument::Array(ref a) => {
let a = Box::new(wl_array {
size: a.len(),
Expand Down
10 changes: 5 additions & 5 deletions wayland-backend/src/sys/server_impl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{
os::raw::{c_int, c_void},
os::unix::io::{BorrowedFd, OwnedFd},
os::unix::{
io::{FromRawFd, IntoRawFd, RawFd},
io::{AsRawFd, FromRawFd, IntoRawFd},
net::UnixStream,
},
sync::{
Expand Down Expand Up @@ -575,7 +575,7 @@ impl InnerHandle {
}
}

pub fn send_event(&self, msg: Message<ObjectId, RawFd>) -> Result<(), InvalidId> {
pub fn send_event(&self, msg: Message<ObjectId, BorrowedFd>) -> Result<(), InvalidId> {
self.state.lock().unwrap().send_event(msg)
}

Expand Down Expand Up @@ -848,7 +848,7 @@ pub(crate) trait ErasedState: downcast_rs::Downcast {
&self,
id: InnerObjectId,
) -> Result<Arc<dyn std::any::Any + Send + Sync>, InvalidId>;
fn send_event(&mut self, msg: Message<ObjectId, RawFd>) -> Result<(), InvalidId>;
fn send_event(&mut self, msg: Message<ObjectId, BorrowedFd>) -> Result<(), InvalidId>;
fn post_error(&mut self, object_id: InnerObjectId, error_code: u32, message: CString);
fn kill_client(&mut self, client_id: InnerClientId, reason: DisconnectReason);
fn global_info(&self, id: InnerGlobalId) -> Result<GlobalInfo, InvalidId>;
Expand Down Expand Up @@ -1048,7 +1048,7 @@ impl<D: 'static> ErasedState for State<D> {

fn send_event(
&mut self,
Message { sender_id: ObjectId { id }, opcode, args }: Message<ObjectId, RawFd>,
Message { sender_id: ObjectId { id }, opcode, args }: Message<ObjectId, BorrowedFd>,
) -> Result<(), InvalidId> {
if !id.alive.load(Ordering::Acquire) || id.ptr.is_null() {
return Err(InvalidId);
Expand All @@ -1075,7 +1075,7 @@ impl<D: 'static> ErasedState for State<D> {
Argument::Uint(u) => argument_list.push(wl_argument { u }),
Argument::Int(i) => argument_list.push(wl_argument { i }),
Argument::Fixed(f) => argument_list.push(wl_argument { f }),
Argument::Fd(h) => argument_list.push(wl_argument { h }),
Argument::Fd(h) => argument_list.push(wl_argument { h: h.as_raw_fd() }),
Argument::Array(ref a) => {
let a = Box::new(wl_array {
size: a.len(),
Expand Down
8 changes: 6 additions & 2 deletions wayland-backend/src/test/many_args.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::{
ffi::{CStr, CString},
io,
os::unix::io::AsFd,
sync::atomic::{AtomicBool, Ordering},
};

Expand Down Expand Up @@ -61,6 +63,7 @@ macro_rules! serverdata_impls {
_: $server_backend::GlobalId,
object_id: $server_backend::ObjectId,
) -> Arc<dyn $server_backend::ObjectData<()>> {
let stdout = io::stdout().lock();
handle
.send_event(message!(
object_id,
Expand All @@ -71,7 +74,7 @@ macro_rules! serverdata_impls {
Argument::Fixed(9823),
Argument::Array(Box::new(vec![10, 20, 30, 40, 50, 60, 70, 80, 90])),
Argument::Str(Some(Box::new(CString::new("I want cake".as_bytes()).unwrap()))),
Argument::Fd(1), // stdout
Argument::Fd(stdout.as_fd()),
],
))
.unwrap();
Expand Down Expand Up @@ -171,6 +174,7 @@ expand_test!(many_args, {
assert!(client_data.0.load(Ordering::SeqCst));

// send the many_args request
let stdin = io::stdin().lock();
client
.send_request(
message!(
Expand All @@ -184,7 +188,7 @@ expand_test!(many_args, {
Argument::Str(Some(Box::new(
CString::new("I like trains".as_bytes()).unwrap()
))),
Argument::Fd(0), // stdin
Argument::Fd(stdin.as_fd()),
],
),
None,
Expand Down
Loading

0 comments on commit a697934

Please sign in to comment.