From ced7739f69498a702640211bfbaeddb7e864b3d2 Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Thu, 16 May 2024 10:30:48 +0200 Subject: [PATCH] tokio: use `ptr::addr_of` instead of pointer arithmetic in linked_list (#6561) --- tokio/src/util/linked_list.rs | 45 ++++++++++------------------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/tokio/src/util/linked_list.rs b/tokio/src/util/linked_list.rs index 0ed2b6164..ab20292e2 100644 --- a/tokio/src/util/linked_list.rs +++ b/tokio/src/util/linked_list.rs @@ -78,26 +78,19 @@ pub(crate) struct Pointers { /// Additionally, we never access the `prev` or `next` fields directly, as any /// such access would implicitly involve the creation of a reference to the /// field, which we want to avoid since the fields are not `!Unpin`, and would -/// hence be given the `noalias` attribute if we were to do such an access. -/// As an alternative to accessing the fields directly, the `Pointers` type +/// hence be given the `noalias` attribute if we were to do such an access. As +/// an alternative to accessing the fields directly, the `Pointers` type /// provides getters and setters for the two fields, and those are implemented -/// using raw pointer casts and offsets, which is valid since the struct is -/// #[repr(C)]. +/// using `ptr`-specific methods which avoids the creation of intermediate +/// references. /// /// See this link for more information: /// -#[repr(C)] struct PointersInner { /// The previous node in the list. null if there is no previous node. - /// - /// This field is accessed through pointer manipulation, so it is not dead code. - #[allow(dead_code)] prev: Option>, /// The next node in the list. null if there is no previous node. - /// - /// This field is accessed through pointer manipulation, so it is not dead code. - #[allow(dead_code)] next: Option>, /// This type is !Unpin due to the heuristic from: @@ -418,38 +411,24 @@ impl Pointers { } pub(crate) fn get_prev(&self) -> Option> { - // SAFETY: prev is the first field in PointersInner, which is #[repr(C)]. - unsafe { - let inner = self.inner.get(); - let prev = inner as *const Option>; - ptr::read(prev) - } + // SAFETY: Field is accessed immutably through a reference. + unsafe { ptr::addr_of!((*self.inner.get()).prev).read() } } pub(crate) fn get_next(&self) -> Option> { - // SAFETY: next is the second field in PointersInner, which is #[repr(C)]. - unsafe { - let inner = self.inner.get(); - let prev = inner as *const Option>; - let next = prev.add(1); - ptr::read(next) - } + // SAFETY: Field is accessed immutably through a reference. + unsafe { ptr::addr_of!((*self.inner.get()).next).read() } } fn set_prev(&mut self, value: Option>) { - // SAFETY: prev is the first field in PointersInner, which is #[repr(C)]. + // SAFETY: Field is accessed mutably through a mutable reference. unsafe { - let inner = self.inner.get(); - let prev = inner as *mut Option>; - ptr::write(prev, value); + ptr::addr_of_mut!((*self.inner.get()).prev).write(value); } } fn set_next(&mut self, value: Option>) { - // SAFETY: next is the second field in PointersInner, which is #[repr(C)]. + // SAFETY: Field is accessed mutably through a mutable reference. unsafe { - let inner = self.inner.get(); - let prev = inner as *mut Option>; - let next = prev.add(1); - ptr::write(next, value); + ptr::addr_of_mut!((*self.inner.get()).next).write(value); } } }