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 13, 2023
1 parent be86c69 commit 3396832
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 36 deletions.
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>;

/// 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
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
23 changes: 16 additions & 7 deletions wayland-scanner/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ pub(crate) fn gen_message_enum(
side: Side,
receiver: bool,
messages: &[Message],
) -> TokenStream {
) -> (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 {
Expand Down Expand Up @@ -199,7 +201,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 @@ -275,7 +278,7 @@ pub(crate) fn gen_message_enum(
#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 +294,28 @@ pub(crate) fn gen_message_enum(
}
});

quote! {
let generic = if needs_generic {
quote! { 'a }
} else {
quote! {}
};

(quote! {
#[derive(Debug)]
#[non_exhaustive]
pub enum #name {
pub enum #name<#generic> {
#(#variants,)*
}

impl #name {
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
5 changes: 3 additions & 2 deletions wayland-server/src/display.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{os::unix::net::UnixStream, sync::Arc};
use std::{os::unix::{io::AsRawFd, net::UnixStream}, sync::Arc};

use wayland_backend::{
protocol::ObjectInfo,
Expand Down Expand Up @@ -161,8 +161,9 @@ impl DisplayHandle {
///
/// This is intended to be a low-level method. You can alternatively use the methods on the
/// type representing your object, or [`Resource::send_event()`], which may be more convenient.
pub fn send_event<I: Resource>(&self, resource: &I, event: I::Event) -> Result<(), InvalidId> {
pub fn send_event<I: Resource>(&self, resource: &I, event: I::Event<'_>) -> Result<(), InvalidId> {
let msg = resource.write_event(self, event)?;
let msg = msg.map_fd(|fd| fd.as_raw_fd());
self.handle.send_event(msg)
}

Expand Down
10 changes: 5 additions & 5 deletions wayland-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ use std::{
/// Trait representing a Wayland interface
pub trait Resource: Clone + std::fmt::Debug + Sized {
/// The event enum for this interface
type Event;
type Event<'a>;
/// The request enum for this interface
type Request;

Expand Down Expand Up @@ -192,7 +192,7 @@ pub trait Resource: Clone + std::fmt::Debug + Sized {
fn from_id(dh: &DisplayHandle, id: ObjectId) -> Result<Self, InvalidId>;

/// Send an event to this object
fn send_event(&self, evt: Self::Event) -> Result<(), InvalidId>;
fn send_event(&self, evt: Self::Event<'_>) -> Result<(), InvalidId>;

/// Trigger a protocol error on this object
///
Expand All @@ -219,11 +219,11 @@ pub trait Resource: Clone + std::fmt::Debug + Sized {
///
/// **Note:** This method is mostly meant as an implementation detail to be used by code generated by
/// wayland-scanner.
fn write_event(
fn write_event<'a>(
&self,
dh: &DisplayHandle,
req: Self::Event,
) -> Result<Message<ObjectId, std::os::unix::io::RawFd>, InvalidId>;
req: Self::Event<'a>,
) -> Result<Message<ObjectId, std::os::unix::io::BorrowedFd<'a>>, InvalidId>;

/// Creates a weak handle to this object
///
Expand Down

0 comments on commit 3396832

Please sign in to comment.