Skip to content

Commit 40839c0

Browse files
committed
rust: use generic associated types (GATs) to implement PointerWrapper.
The feature, although still unstable, is no longer incomplete. It allows us to use a lifetime in the `Borrow` assocciated type, which allows us to simplify some of the code around borrowing the wrapped value and also allows us to wrap raw pointers and integers without violating `Deref` recommendations. Signed-off-by: Wedson Almeida Filho <[email protected]>
1 parent 0f3ae60 commit 40839c0

File tree

12 files changed

+165
-136
lines changed

12 files changed

+165
-136
lines changed

drivers/android/process.rs

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use kernel::{
1010
pages::Pages,
1111
prelude::*,
1212
rbtree::RBTree,
13-
sync::{GuardMut, Mutex, Ref, UniqueRef},
13+
sync::{GuardMut, Mutex, Ref, RefBorrow, UniqueRef},
1414
task::Task,
1515
user_ptr::{UserSlicePtr, UserSlicePtrReader},
1616
};
@@ -307,7 +307,7 @@ impl Process {
307307
Either::Right(Registration::new(self, thread, &mut inner))
308308
}
309309

310-
fn get_thread(self: &Ref<Self>, id: i32) -> Result<Ref<Thread>> {
310+
fn get_thread(self: RefBorrow<'_, Self>, id: i32) -> Result<Ref<Thread>> {
311311
// TODO: Consider using read/write locks here instead.
312312
{
313313
let inner = self.inner.lock();
@@ -317,7 +317,7 @@ impl Process {
317317
}
318318

319319
// Allocate a new `Thread` without holding any locks.
320-
let ta = Thread::new(id, self.clone())?;
320+
let ta = Thread::new(id, self.into())?;
321321
let node = RBTree::try_allocate_node(id, ta.clone())?;
322322

323323
let mut inner = self.inner.lock();
@@ -335,7 +335,11 @@ impl Process {
335335
self.inner.lock().push_work(work)
336336
}
337337

338-
fn set_as_manager(self: &Ref<Self>, info: Option<FlatBinderObject>, thread: &Thread) -> Result {
338+
fn set_as_manager(
339+
self: RefBorrow<'_, Self>,
340+
info: Option<FlatBinderObject>,
341+
thread: &Thread,
342+
) -> Result {
339343
let (ptr, cookie, flags) = if let Some(obj) = info {
340344
(
341345
// SAFETY: The object type for this ioctl is implicitly `BINDER_TYPE_BINDER`, so it
@@ -359,7 +363,7 @@ impl Process {
359363
}
360364

361365
pub(crate) fn get_node(
362-
self: &Ref<Self>,
366+
self: RefBorrow<'_, Self>,
363367
ptr: usize,
364368
cookie: usize,
365369
flags: u32,
@@ -375,7 +379,7 @@ impl Process {
375379
}
376380

377381
// Allocate the node before reacquiring the lock.
378-
let node = Ref::try_new(Node::new(ptr, cookie, flags, self.clone()))?;
382+
let node = Ref::try_new(Node::new(ptr, cookie, flags, self.into()))?;
379383
let rbnode = RBTree::try_allocate_node(ptr, node.clone())?;
380384

381385
let mut inner = self.inner.lock();
@@ -758,10 +762,10 @@ impl Process {
758762
}
759763

760764
impl IoctlHandler for Process {
761-
type Target = Ref<Process>;
765+
type Target<'a> = RefBorrow<'a, Process>;
762766

763767
fn write(
764-
this: &Ref<Process>,
768+
this: RefBorrow<'_, Process>,
765769
_file: &File,
766770
cmd: u32,
767771
reader: &mut UserSlicePtrReader,
@@ -779,7 +783,12 @@ impl IoctlHandler for Process {
779783
Ok(0)
780784
}
781785

782-
fn read_write(this: &Ref<Process>, file: &File, cmd: u32, data: UserSlicePtr) -> Result<i32> {
786+
fn read_write(
787+
this: RefBorrow<'_, Process>,
788+
file: &File,
789+
cmd: u32,
790+
data: UserSlicePtr,
791+
) -> Result<i32> {
783792
let thread = this.get_thread(Task::current().pid())?;
784793
match cmd {
785794
bindings::BINDER_WRITE_READ => thread.write_read(data, file.is_blocking())?,
@@ -879,15 +888,23 @@ impl FileOperations for Process {
879888
}
880889
}
881890

882-
fn ioctl(this: &Ref<Process>, file: &File, cmd: &mut IoctlCommand) -> Result<i32> {
891+
fn ioctl(this: RefBorrow<'_, Process>, file: &File, cmd: &mut IoctlCommand) -> Result<i32> {
883892
cmd.dispatch::<Self>(this, file)
884893
}
885894

886-
fn compat_ioctl(this: &Ref<Process>, file: &File, cmd: &mut IoctlCommand) -> Result<i32> {
895+
fn compat_ioctl(
896+
this: RefBorrow<'_, Process>,
897+
file: &File,
898+
cmd: &mut IoctlCommand,
899+
) -> Result<i32> {
887900
cmd.dispatch::<Self>(this, file)
888901
}
889902

890-
fn mmap(this: &Ref<Process>, _file: &File, vma: &mut bindings::vm_area_struct) -> Result {
903+
fn mmap(
904+
this: RefBorrow<'_, Process>,
905+
_file: &File,
906+
vma: &mut bindings::vm_area_struct,
907+
) -> Result {
891908
// We don't allow mmap to be used in a different process.
892909
if !Task::current().group_leader().eq(&this.task) {
893910
return Err(Error::EINVAL);
@@ -908,7 +925,7 @@ impl FileOperations for Process {
908925
this.create_mapping(vma)
909926
}
910927

911-
fn poll(this: &Ref<Process>, file: &File, table: &PollTable) -> Result<u32> {
928+
fn poll(this: RefBorrow<'_, Process>, file: &File, table: &PollTable) -> Result<u32> {
912929
let thread = this.get_thread(Task::current().pid())?;
913930
let (from_proc, mut mask) = thread.poll(file, table);
914931
if mask == 0 && from_proc && !this.inner.lock().work.is_empty() {

drivers/android/rust_binder.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@
55
//! TODO: This module is a work in progress.
66
77
#![no_std]
8-
#![feature(global_asm, try_reserve, allocator_api, concat_idents)]
8+
#![feature(
9+
global_asm,
10+
try_reserve,
11+
allocator_api,
12+
concat_idents,
13+
generic_associated_types
14+
)]
915

1016
use kernel::{
1117
io_buffer::IoBufferWriter,

drivers/android/thread.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -395,9 +395,13 @@ impl Thread {
395395
let ptr = unsafe { obj.__bindgen_anon_1.binder } as _;
396396
let cookie = obj.cookie as _;
397397
let flags = obj.flags as _;
398-
let node = self
399-
.process
400-
.get_node(ptr, cookie, flags, strong, Some(self))?;
398+
let node = self.process.as_ref_borrow().get_node(
399+
ptr,
400+
cookie,
401+
flags,
402+
strong,
403+
Some(self),
404+
)?;
401405
security::binder_transfer_binder(&self.process.task, &view.alloc.process.task)?;
402406
Ok(node)
403407
})?;

rust/kernel/file_operations.rs

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
//! C header: [`include/linux/fs.h`](../../../../include/linux/fs.h)
66
77
use core::convert::{TryFrom, TryInto};
8-
use core::{marker, mem, ops::Deref, ptr};
8+
use core::{marker, mem, ptr};
99

1010
use alloc::boxed::Box;
1111

@@ -122,7 +122,7 @@ unsafe extern "C" fn read_callback<T: FileOperations>(
122122
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
123123
// No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63).
124124
// See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113
125-
let read = T::read(&f, unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?;
125+
let read = T::read(f, unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?;
126126
unsafe { (*offset) += bindings::loff_t::try_from(read).unwrap() };
127127
Ok(read as _)
128128
}
@@ -141,7 +141,7 @@ unsafe extern "C" fn read_iter_callback<T: FileOperations>(
141141
// callback, which the C API guarantees that will be called only when all references to
142142
// `file` have been released, so we know it can't be called while this function is running.
143143
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
144-
let read = T::read(&f, unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?;
144+
let read = T::read(f, unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?;
145145
unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(read).unwrap() };
146146
Ok(read as _)
147147
}
@@ -162,7 +162,7 @@ unsafe extern "C" fn write_callback<T: FileOperations>(
162162
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
163163
// No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63).
164164
// See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113
165-
let written = T::write(&f, unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?;
165+
let written = T::write(f, unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?;
166166
unsafe { (*offset) += bindings::loff_t::try_from(written).unwrap() };
167167
Ok(written as _)
168168
}
@@ -181,7 +181,7 @@ unsafe extern "C" fn write_iter_callback<T: FileOperations>(
181181
// callback, which the C API guarantees that will be called only when all references to
182182
// `file` have been released, so we know it can't be called while this function is running.
183183
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
184-
let written = T::write(&f, unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?;
184+
let written = T::write(f, unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?;
185185
unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(written).unwrap() };
186186
Ok(written as _)
187187
}
@@ -215,7 +215,7 @@ unsafe extern "C" fn llseek_callback<T: FileOperations>(
215215
// callback, which the C API guarantees that will be called only when all references to
216216
// `file` have been released, so we know it can't be called while this function is running.
217217
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
218-
let off = T::seek(&f, unsafe { &FileRef::from_ptr(file) }, off)?;
218+
let off = T::seek(f, unsafe { &FileRef::from_ptr(file) }, off)?;
219219
Ok(off as bindings::loff_t)
220220
}
221221
}
@@ -232,7 +232,7 @@ unsafe extern "C" fn unlocked_ioctl_callback<T: FileOperations>(
232232
// `file` have been released, so we know it can't be called while this function is running.
233233
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
234234
let mut cmd = IoctlCommand::new(cmd as _, arg as _);
235-
let ret = T::ioctl(&f, unsafe { &FileRef::from_ptr(file) }, &mut cmd)?;
235+
let ret = T::ioctl(f, unsafe { &FileRef::from_ptr(file) }, &mut cmd)?;
236236
Ok(ret as _)
237237
}
238238
}
@@ -249,7 +249,7 @@ unsafe extern "C" fn compat_ioctl_callback<T: FileOperations>(
249249
// `file` have been released, so we know it can't be called while this function is running.
250250
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
251251
let mut cmd = IoctlCommand::new(cmd as _, arg as _);
252-
let ret = T::compat_ioctl(&f, unsafe { &FileRef::from_ptr(file) }, &mut cmd)?;
252+
let ret = T::compat_ioctl(f, unsafe { &FileRef::from_ptr(file) }, &mut cmd)?;
253253
Ok(ret as _)
254254
}
255255
}
@@ -264,7 +264,7 @@ unsafe extern "C" fn mmap_callback<T: FileOperations>(
264264
// callback, which the C API guarantees that will be called only when all references to
265265
// `file` have been released, so we know it can't be called while this function is running.
266266
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
267-
T::mmap(&f, unsafe { &FileRef::from_ptr(file) }, unsafe { &mut *vma })?;
267+
T::mmap(f, unsafe { &FileRef::from_ptr(file) }, unsafe { &mut *vma })?;
268268
Ok(0)
269269
}
270270
}
@@ -284,7 +284,7 @@ unsafe extern "C" fn fsync_callback<T: FileOperations>(
284284
// callback, which the C API guarantees that will be called only when all references to
285285
// `file` have been released, so we know it can't be called while this function is running.
286286
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
287-
let res = T::fsync(&f, unsafe { &FileRef::from_ptr(file) }, start, end, datasync)?;
287+
let res = T::fsync(f, unsafe { &FileRef::from_ptr(file) }, start, end, datasync)?;
288288
Ok(res.try_into().unwrap())
289289
}
290290
}
@@ -298,7 +298,7 @@ unsafe extern "C" fn poll_callback<T: FileOperations>(
298298
// callback, which the C API guarantees that will be called only when all references to `file`
299299
// have been released, so we know it can't be called while this function is running.
300300
let f = unsafe { T::Wrapper::borrow((*file).private_data) };
301-
match T::poll(&f, unsafe { &FileRef::from_ptr(file) }, unsafe {
301+
match T::poll(f, unsafe { &FileRef::from_ptr(file) }, unsafe {
302302
&PollTable::from_ptr(wait)
303303
}) {
304304
Ok(v) => v,
@@ -463,17 +463,17 @@ macro_rules! declare_file_operations {
463463
/// For each macro, there is a handler function that takes the appropriate types as arguments.
464464
pub trait IoctlHandler: Sync {
465465
/// The type of the first argument to each associated function.
466-
type Target;
466+
type Target<'a>;
467467

468468
/// Handles ioctls defined with the `_IO` macro, that is, with no buffer as argument.
469-
fn pure(_this: &Self::Target, _file: &File, _cmd: u32, _arg: usize) -> Result<i32> {
469+
fn pure(_this: Self::Target<'_>, _file: &File, _cmd: u32, _arg: usize) -> Result<i32> {
470470
Err(Error::EINVAL)
471471
}
472472

473473
/// Handles ioctls defined with the `_IOR` macro, that is, with an output buffer provided as
474474
/// argument.
475475
fn read(
476-
_this: &Self::Target,
476+
_this: Self::Target<'_>,
477477
_file: &File,
478478
_cmd: u32,
479479
_writer: &mut UserSlicePtrWriter,
@@ -484,7 +484,7 @@ pub trait IoctlHandler: Sync {
484484
/// Handles ioctls defined with the `_IOW` macro, that is, with an input buffer provided as
485485
/// argument.
486486
fn write(
487-
_this: &Self::Target,
487+
_this: Self::Target<'_>,
488488
_file: &File,
489489
_cmd: u32,
490490
_reader: &mut UserSlicePtrReader,
@@ -495,7 +495,7 @@ pub trait IoctlHandler: Sync {
495495
/// Handles ioctls defined with the `_IOWR` macro, that is, with a buffer for both input and
496496
/// output provided as argument.
497497
fn read_write(
498-
_this: &Self::Target,
498+
_this: Self::Target<'_>,
499499
_file: &File,
500500
_cmd: u32,
501501
_data: UserSlicePtr,
@@ -535,7 +535,11 @@ impl IoctlCommand {
535535
///
536536
/// It is meant to be used in implementations of [`FileOperations::ioctl`] and
537537
/// [`FileOperations::compat_ioctl`].
538-
pub fn dispatch<T: IoctlHandler>(&mut self, handler: &T::Target, file: &File) -> Result<i32> {
538+
pub fn dispatch<T: IoctlHandler>(
539+
&mut self,
540+
handler: T::Target<'_>,
541+
file: &File,
542+
) -> Result<i32> {
539543
let dir = (self.cmd >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK;
540544
if dir == bindings::_IOC_NONE {
541545
return T::pure(handler, file, self.cmd, self.arg);
@@ -604,7 +608,7 @@ impl<T: FileOperations<Wrapper = Box<T>> + Default> FileOpener<()> for T {
604608
/// File descriptors may be used from multiple threads/processes concurrently, so your type must be
605609
/// [`Sync`]. It must also be [`Send`] because [`FileOperations::release`] will be called from the
606610
/// thread that decrements that associated file's refcount to zero.
607-
pub trait FileOperations: Send + Sync + Sized {
611+
pub trait FileOperations: Send + Sync + Sized + 'static {
608612
/// The methods to use to populate [`struct file_operations`].
609613
const TO_USE: ToUse;
610614

@@ -623,7 +627,7 @@ pub trait FileOperations: Send + Sync + Sized {
623627
///
624628
/// Corresponds to the `read` and `read_iter` function pointers in `struct file_operations`.
625629
fn read(
626-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
630+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
627631
_file: &File,
628632
_data: &mut impl IoBufferWriter,
629633
_offset: u64,
@@ -635,7 +639,7 @@ pub trait FileOperations: Send + Sync + Sized {
635639
///
636640
/// Corresponds to the `write` and `write_iter` function pointers in `struct file_operations`.
637641
fn write(
638-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
642+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
639643
_file: &File,
640644
_data: &mut impl IoBufferReader,
641645
_offset: u64,
@@ -647,7 +651,7 @@ pub trait FileOperations: Send + Sync + Sized {
647651
///
648652
/// Corresponds to the `llseek` function pointer in `struct file_operations`.
649653
fn seek(
650-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
654+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
651655
_file: &File,
652656
_offset: SeekFrom,
653657
) -> Result<u64> {
@@ -658,7 +662,7 @@ pub trait FileOperations: Send + Sync + Sized {
658662
///
659663
/// Corresponds to the `unlocked_ioctl` function pointer in `struct file_operations`.
660664
fn ioctl(
661-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
665+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
662666
_file: &File,
663667
_cmd: &mut IoctlCommand,
664668
) -> Result<i32> {
@@ -669,7 +673,7 @@ pub trait FileOperations: Send + Sync + Sized {
669673
///
670674
/// Corresponds to the `compat_ioctl` function pointer in `struct file_operations`.
671675
fn compat_ioctl(
672-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
676+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
673677
_file: &File,
674678
_cmd: &mut IoctlCommand,
675679
) -> Result<i32> {
@@ -680,7 +684,7 @@ pub trait FileOperations: Send + Sync + Sized {
680684
///
681685
/// Corresponds to the `fsync` function pointer in `struct file_operations`.
682686
fn fsync(
683-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
687+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
684688
_file: &File,
685689
_start: u64,
686690
_end: u64,
@@ -694,7 +698,7 @@ pub trait FileOperations: Send + Sync + Sized {
694698
/// Corresponds to the `mmap` function pointer in `struct file_operations`.
695699
/// TODO: wrap `vm_area_struct` so that we don't have to expose it.
696700
fn mmap(
697-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
701+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
698702
_file: &File,
699703
_vma: &mut bindings::vm_area_struct,
700704
) -> Result {
@@ -706,7 +710,7 @@ pub trait FileOperations: Send + Sync + Sized {
706710
///
707711
/// Corresponds to the `poll` function pointer in `struct file_operations`.
708712
fn poll(
709-
_this: &<<Self::Wrapper as PointerWrapper>::Borrowed as Deref>::Target,
713+
_this: <Self::Wrapper as PointerWrapper>::Borrowed<'_>,
710714
_file: &File,
711715
_table: &PollTable,
712716
) -> Result<u32> {

0 commit comments

Comments
 (0)