Skip to content

Commit

Permalink
WIP use BorrowedFd in arguments
Browse files Browse the repository at this point in the history
Implementation of #592.

This is required for IO safety. Or perhaps `AsFd` generics would be
useful.

This will be a breaking API change, and require Rust 1.65.

Currently `wayland-client` compiles here, but server-side needs the same
changes, and things need to be cleaned up a bit.
  • Loading branch information
ids1024 committed Jul 14, 2023
1 parent 5774a01 commit 3955b21
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 83 deletions.
5 changes: 2 additions & 3 deletions wayland-client/examples/simple_window.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{fs::File, os::unix::prelude::AsRawFd};
use std::{fs::File, os::unix::prelude::AsFd};

use wayland_client::{
delegate_noop,
Expand Down Expand Up @@ -73,8 +73,7 @@ impl Dispatch<wl_registry::WlRegistry, ()> for State {

let mut file = tempfile::tempfile().unwrap();
draw(&mut file, (init_w, init_h));
let pool =
shm.create_pool(file.as_raw_fd(), (init_w * init_h * 4) as i32, qh, ());
let pool = shm.create_pool(file.as_fd(), (init_w * init_h * 4) as i32, qh, ());
let buffer = pool.create_buffer(
0,
init_w as i32,
Expand Down
3 changes: 2 additions & 1 deletion wayland-client/src/conn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,11 @@ impl Connection {
pub fn send_request<I: Proxy>(
&self,
proxy: &I,
request: I::Request,
request: I::Request<'_>,
data: Option<Arc<dyn ObjectData>>,
) -> Result<ObjectId, InvalidId> {
let (msg, child_spec) = proxy.write_request(self, request)?;
let msg = msg.map_fd(|fd| fd.as_raw_fd());
self.backend.send_request(msg, data, child_spec)
}

Expand Down
15 changes: 7 additions & 8 deletions wayland-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,11 @@
use std::{
fmt,
hash::{Hash, Hasher},
os::unix::io::RawFd,
sync::Arc,
};
use wayland_backend::{
client::{InvalidId, ObjectData, ObjectId, WaylandError, WeakBackend},
io_lifetimes::OwnedFd,
io_lifetimes::{BorrowedFd, OwnedFd},
protocol::{Interface, Message},
};

Expand Down Expand Up @@ -227,7 +226,7 @@ pub trait Proxy: Clone + std::fmt::Debug + Sized {
/// The event enum for this interface
type Event;
/// The request enum for this interface
type Request;
type Request<'a>;

Check failure on line 229 in wayland-client/src/lib.rs

View workflow job for this annotation

GitHub Actions / msrv

generic associated types are unstable

/// The interface description
fn interface() -> &'static Interface;
Expand Down Expand Up @@ -278,15 +277,15 @@ pub trait Proxy: Clone + std::fmt::Debug + Sized {
///
/// It is an error to use this function on requests that create objects; use
/// [Proxy::send_constructor] for such requests.
fn send_request(&self, req: Self::Request) -> Result<(), InvalidId>;
fn send_request(&self, req: Self::Request<'_>) -> Result<(), InvalidId>;

/// Send a request for this object that creates another object.
///
/// It is an error to use this function on requests that do not create objects; use
/// [Proxy::send_request] for such requests.
fn send_constructor<I: Proxy>(
&self,
req: Self::Request,
req: Self::Request<'_>,
data: Arc<dyn ObjectData>,
) -> Result<I, InvalidId>;

Expand All @@ -304,11 +303,11 @@ pub trait Proxy: Clone + std::fmt::Debug + Sized {
/// **Note:** This method is mostly meant as an implementation detail to be
/// used by code generated by wayland-scanner.
#[allow(clippy::type_complexity)]
fn write_request(
fn write_request<'a>(
&self,
conn: &Connection,
req: Self::Request,
) -> Result<(Message<ObjectId, RawFd>, Option<(&'static Interface, u32)>), InvalidId>;
req: Self::Request<'a>,
) -> Result<(Message<ObjectId, BorrowedFd<'a>>, Option<(&'static Interface, u32)>), InvalidId>;

/// Creates a weak handle to this object
///
Expand Down
4 changes: 2 additions & 2 deletions wayland-cursor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use std::env;
use std::fs::File;
use std::io::{Error as IoError, Read, Result as IoResult, Seek, SeekFrom, Write};
use std::ops::{Deref, Index};
use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
use std::os::unix::io::{AsFd, FromRawFd, RawFd};
use std::sync::Arc;
use std::time::{SystemTime, UNIX_EPOCH};

Expand Down Expand Up @@ -145,7 +145,7 @@ impl CursorTheme {

let pool_id = conn.send_request(
&shm,
wl_shm::Request::CreatePool { fd: file.as_raw_fd(), size: INITIAL_POOL_SIZE },
wl_shm::Request::CreatePool { fd: file.as_fd(), size: INITIAL_POOL_SIZE },
Some(Arc::new(IgnoreObjectData)),
)?;
let pool = WlShmPool::from_id(conn, pool_id)?;
Expand Down
20 changes: 13 additions & 7 deletions wayland-scanner/src/client_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
let enums = crate::common::generate_enums_for(interface);
let sinces = crate::common::gen_msg_constants(&interface.requests, &interface.events);

let requests = crate::common::gen_message_enum(
let (requests, requests_is_generic) = crate::common::gen_message_enum(
&format_ident!("Request"),
Side::Client,
false,
&interface.requests,
);
let events = crate::common::gen_message_enum(
let (events, _) = crate::common::gen_message_enum(
&format_ident!("Event"),
Side::Client,
true,
Expand All @@ -49,6 +49,12 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
};
let doc_attr = to_doc_attr(&docs);

let request_generic = if requests_is_generic {
quote! { 'request }
} else {
quote! {}
};

quote! {
#mod_doc
pub mod #mod_name {
Expand Down Expand Up @@ -103,7 +109,7 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
}

impl super::wayland_client::Proxy for #iface_name {
type Request = Request;
type Request<'request> = Request<#request_generic>;
type Event = Event;

#[inline]
Expand Down Expand Up @@ -134,14 +140,14 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
&self.backend
}

fn send_request(&self, req: Self::Request) -> Result<(), InvalidId> {
fn send_request(&self, req: Self::Request<'_>) -> Result<(), InvalidId> {
let conn = Connection::from_backend(self.backend.upgrade().ok_or(InvalidId)?);
let id = conn.send_request(self, req, None)?;
debug_assert!(id.is_null());
Ok(())
}

fn send_constructor<I: Proxy>(&self, req: Self::Request, data: Arc<dyn ObjectData>) -> Result<I, InvalidId> {
fn send_constructor<I: Proxy>(&self, req: Self::Request<'_>, data: Arc<dyn ObjectData>) -> Result<I, InvalidId> {
let conn = Connection::from_backend(self.backend.upgrade().ok_or(InvalidId)?);
let id = conn.send_request(self, req, Some(data))?;
Proxy::from_id(&conn, id)
Expand All @@ -167,7 +173,7 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
#parse_body
}

fn write_request(&self, conn: &Connection, msg: Self::Request) -> Result<(Message<ObjectId, std::os::unix::io::RawFd>, Option<(&'static Interface, u32)>), InvalidId> {
fn write_request<'a>(&self, conn: &Connection, msg: Self::Request<'a>) -> Result<(Message<ObjectId, io_lifetimes::BorrowedFd<'a>>, Option<(&'static Interface, u32)>), InvalidId> {
#write_body
}
}
Expand Down Expand Up @@ -211,7 +217,7 @@ fn gen_methods(interface: &Interface) -> TokenStream {
Type::Fixed => quote! { f64 },
Type::String => if arg.allow_null { quote!{ Option<String> } } else { quote!{ String } },
Type::Array => if arg.allow_null { quote!{ Option<Vec<u8>> } } else { quote!{ Vec<u8> } },
Type::Fd => quote! { ::std::os::unix::io::RawFd },
Type::Fd => quote! { ::std::os::unix::io::BorrowedFd<'_> },
Type::Object => {
let iface = arg.interface.as_ref().unwrap();
let iface_mod = Ident::new(iface, Span::call_site());
Expand Down
111 changes: 64 additions & 47 deletions wayland-scanner/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,30 +159,36 @@ pub(crate) fn gen_message_enum(
side: Side,
receiver: bool,
messages: &[Message],
) -> TokenStream {
let variants = messages.iter().map(|msg| {
let mut docs = String::new();
if let Some((ref short, ref long)) = msg.description {
write!(docs, "{}\n\n{}\n", short, long.trim()).unwrap();
}
if let Some(Type::Destructor) = msg.typ {
write!(
docs,
"\nThis is a destructor, once {} this object cannot be used any longer.",
if receiver { "received" } else { "sent" },
)
.unwrap()
}
if msg.since > 1 {
write!(docs, "\nOnly available since version {} of the interface", msg.since).unwrap();
}
) -> (TokenStream, bool) {
let mut needs_generic = false;

let variants = messages
.iter()
.map(|msg| {
let mut docs = String::new();
if let Some((ref short, ref long)) = msg.description {
write!(docs, "{}\n\n{}\n", short, long.trim()).unwrap();
}
if let Some(Type::Destructor) = msg.typ {
write!(
docs,
"\nThis is a destructor, once {} this object cannot be used any longer.",
if receiver { "received" } else { "sent" },
)
.unwrap()
}
if msg.since > 1 {
write!(docs, "\nOnly available since version {} of the interface", msg.since)
.unwrap();
}

let doc_attr = to_doc_attr(&docs);
let msg_name = Ident::new(&snake_to_camel(&msg.name), Span::call_site());
let msg_variant_decl = if msg.args.is_empty() {
msg_name.into_token_stream()
} else {
let fields = msg.args.iter().flat_map(|arg| {
let doc_attr = to_doc_attr(&docs);
let msg_name = Ident::new(&snake_to_camel(&msg.name), Span::call_site());
let msg_variant_decl =
if msg.args.is_empty() {
msg_name.into_token_stream()
} else {
let fields = msg.args.iter().flat_map(|arg| {
let field_name =
format_ident!("{}{}", if is_keyword(&arg.name) { "_" } else { "" }, arg.name);
let field_type_inner = if let Some(ref enu) = arg.enum_ {
Expand All @@ -199,7 +205,8 @@ pub(crate) fn gen_message_enum(
if receiver {
quote! { io_lifetimes::OwnedFd }
} else {
quote! { std::os::unix::io::RawFd }
needs_generic = true;
quote! { io_lifetimes::BorrowedFd<'a> }
}
}
Type::Object => {
Expand Down Expand Up @@ -264,18 +271,19 @@ pub(crate) fn gen_message_enum(
})
});

quote! {
#msg_name {
#(#fields,)*
}
}
};

quote! {
#msg_name {
#(#fields,)*
}
#doc_attr
#msg_variant_decl
}
};

quote! {
#doc_attr
#msg_variant_decl
}
});
})
.collect::<Vec<_>>();

let opcodes = messages.iter().enumerate().map(|(opcode, msg)| {
let msg_name = Ident::new(&snake_to_camel(&msg.name), Span::call_site());
Expand All @@ -291,22 +299,31 @@ pub(crate) fn gen_message_enum(
}
});

quote! {
#[derive(Debug)]
#[non_exhaustive]
pub enum #name {
#(#variants,)*
}
let generic = if needs_generic {
quote! { 'a }
} else {
quote! {}
};

impl #name {
#[doc="Get the opcode number of this message"]
pub fn opcode(&self) -> u16 {
match *self {
#(#opcodes,)*
(
quote! {
#[derive(Debug)]
#[non_exhaustive]
pub enum #name<#generic> {
#(#variants,)*
}

impl<#generic> #name<#generic> {
#[doc="Get the opcode number of this message"]
pub fn opcode(&self) -> u16 {
match *self {
#(#opcodes,)*
}
}
}
}
}
},
needs_generic,
)
}

pub(crate) fn gen_parse_body(interface: &Interface, side: Side) -> TokenStream {
Expand Down
18 changes: 12 additions & 6 deletions wayland-scanner/src/server_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
let enums = crate::common::generate_enums_for(interface);
let msg_constants = crate::common::gen_msg_constants(&interface.requests, &interface.events);

let requests = crate::common::gen_message_enum(
let (requests, _) = crate::common::gen_message_enum(
&format_ident!("Request"),
Side::Server,
true,
&interface.requests,
);
let events = crate::common::gen_message_enum(
let (events, events_is_generic) = crate::common::gen_message_enum(
&format_ident!("Event"),
Side::Server,
false,
Expand All @@ -54,6 +54,12 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
};
let doc_attr = to_doc_attr(&docs);

let event_generic = if events_is_generic {
quote! { 'event }
} else {
quote! {}
};

quote! {
#mod_doc
pub mod #mod_name {
Expand Down Expand Up @@ -109,7 +115,7 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {

impl super::wayland_server::Resource for #iface_name {
type Request = Request;
type Event = Event;
type Event<'event> = Event<#event_generic>;

#[inline]
fn interface() -> &'static Interface{
Expand Down Expand Up @@ -150,7 +156,7 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
Ok(#iface_name { id, data, version, handle: conn.backend_handle().downgrade() })
}

fn send_event(&self, evt: Self::Event) -> Result<(), InvalidId> {
fn send_event(&self, evt: Self::Event<'_>) -> Result<(), InvalidId> {
let handle = DisplayHandle::from(self.handle.upgrade().ok_or(InvalidId)?);
handle.send_event(self, evt)
}
Expand All @@ -159,7 +165,7 @@ fn generate_objects_for(interface: &Interface) -> TokenStream {
#parse_body
}

fn write_event(&self, conn: &DisplayHandle, msg: Self::Event) -> Result<Message<ObjectId, std::os::unix::io::RawFd>, InvalidId> {
fn write_event<'a>(&self, conn: &DisplayHandle, msg: Self::Event<'a>) -> Result<Message<ObjectId, std::os::unix::io::BorrowedFd<'a>>, InvalidId> {
#write_body
}

Expand Down Expand Up @@ -213,7 +219,7 @@ fn gen_methods(interface: &Interface) -> TokenStream {
quote! { Vec<u8> }
}
}
Type::Fd => quote! { ::std::os::unix::io::RawFd },
Type::Fd => quote! { ::std::os::unix::io::BorrowedFd<'_> },
Type::Object | Type::NewId => {
let iface = arg.interface.as_ref().unwrap();
let iface_mod = Ident::new(iface, Span::call_site());
Expand Down
Loading

0 comments on commit 3955b21

Please sign in to comment.