2023-05-17 20:16:50

by Alice Ryhl

Subject: [PATCH v1 1/2] rust: sync: add `Arc::ptr_eq`

Add a method for comparing whether two `Arc` pointers reference the same
underlying object.

This comparison can already be done by getting a reference to the inner
values and comparing whether the references have the same address.
However, writing `Arc::ptr_eq(a, b)` is generally less error-prone than
doing the same check on the references, since you might otherwise
accidentally compare the two `&Arc<T>` references instead, which wont
work because those are pointers to pointers to the inner value, when you
just want to compare the pointers to the inner value.

Also, this method might optimize better because getting a reference to
the inner value involves offsetting the pointer, which this method does
not need to do.

Co-developed-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Alice Ryhl <[email protected]>
---
rust/kernel/sync/arc.rs | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index e6d206242465..274febe3bb06 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -221,6 +221,11 @@ impl<T: ?Sized> Arc<T> {
// reference can be created.
unsafe { ArcBorrow::new(self.ptr) }
}
+
+ /// Compare whether two [`Arc`] pointers reference the same underlying object.
+ pub fn ptr_eq(this: &Self, other: &Self) -> bool {
+ core::ptr::eq(this.ptr.as_ptr(), other.ptr.as_ptr())
+ }
}

impl<T: 'static> ForeignOwnable for Arc<T> {

base-commit: ac9a78681b921877518763ba0e89202254349d1b
--
2.40.1.606.ga4b1b128d6-goog

2023-05-17 20:22:14

by Alice Ryhl

Subject: [PATCH v1 2/2] rust: sync: implement `AsRef<T>` for `Arc<T>`

This trait lets you use `Arc<T>` in code that is generic over smart
pointer types.

The `AsRef` trait should be implemented on all smart pointers. The
standard library also implements it on the ordinary `Arc`.

Co-developed-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Wedson Almeida Filho <[email protected]>
Signed-off-by: Alice Ryhl <[email protected]>
---
rust/kernel/sync/arc.rs | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 274febe3bb06..9ec911e4a0c7 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -264,6 +264,12 @@ impl<T: ?Sized> Deref for Arc<T> {
}
}

+impl<T: ?Sized> AsRef<T> for Arc<T> {
+ fn as_ref(&self) -> &T {
+ self.deref()
+ }
+}
+
impl<T: ?Sized> Clone for Arc<T> {
fn clone(&self) -> Self {
// INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
--
2.40.1.606.ga4b1b128d6-goog

2023-05-17 21:03:50

by Martin Rodriguez Reboredo

Subject: Re: [PATCH v1 1/2] rust: sync: add `Arc::ptr_eq`

On 5/17/23 17:08, Alice Ryhl wrote:
> [...]
> +
> + /// Compare whether two [`Arc`] pointers reference the same underlying object.
> + pub fn ptr_eq(this: &Self, other: &Self) -> bool {
> + core::ptr::eq(this.ptr.as_ptr(), other.ptr.as_ptr())
> + }
> }
>
> impl<T: 'static> ForeignOwnable for Arc<T> {
>
> base-commit: ac9a78681b921877518763ba0e89202254349d1b

Reviewed-by: Martin Rodriguez Reboredo <[email protected]>

2023-05-17 21:09:39

by Martin Rodriguez Reboredo

Subject: Re: [PATCH v1 2/2] rust: sync: implement `AsRef<T>` for `Arc<T>`

On 5/17/23 17:08, Alice Ryhl wrote:
> [...]
>
> +impl<T: ?Sized> AsRef<T> for Arc<T> {
> + fn as_ref(&self) -> &T {
> + self.deref()
> + }
> +}
> +
> impl<T: ?Sized> Clone for Arc<T> {
> fn clone(&self) -> Self {
> // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.

Reviewed-by: Martin Rodriguez Reboredo <[email protected]>

2023-05-23 12:19:16

by Andreas Hindborg

Subject: Re: [PATCH v1 2/2] rust: sync: implement `AsRef<T>` for `Arc<T>`

Alice Ryhl <[email protected]> writes:

> This trait lets you use `Arc<T>` in code that is generic over smart
> pointer types.
>
> The `AsRef` trait should be implemented on all smart pointers. The
> standard library also implements it on the ordinary `Arc`.
>
> Co-developed-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>

Reviewed-by: Andreas Hindborg <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 274febe3bb06..9ec911e4a0c7 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -264,6 +264,12 @@ impl<T: ?Sized> Deref for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> AsRef<T> for Arc<T> {
> + fn as_ref(&self) -> &T {
> + self.deref()
> + }
> +}
> +
> impl<T: ?Sized> Clone for Arc<T> {
> fn clone(&self) -> Self {
> // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.

2023-05-23 12:38:00

by Andreas Hindborg

Subject: Re: [PATCH v1 1/2] rust: sync: add `Arc::ptr_eq`

Alice Ryhl <[email protected]> writes:

> Add a method for comparing whether two `Arc` pointers reference the same
> underlying object.
>
> This comparison can already be done by getting a reference to the inner
> values and comparing whether the references have the same address.
> However, writing `Arc::ptr_eq(a, b)` is generally less error-prone than
> doing the same check on the references, since you might otherwise
> accidentally compare the two `&Arc<T>` references instead, which wont
> work because those are pointers to pointers to the inner value, when you
> just want to compare the pointers to the inner value.
>
> Also, this method might optimize better because getting a reference to
> the inner value involves offsetting the pointer, which this method does
> not need to do.
>
> Co-developed-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>

Reviewed-by: Andreas Hindborg <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e6d206242465..274febe3bb06 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -221,6 +221,11 @@ impl<T: ?Sized> Arc<T> {
> // reference can be created.
> unsafe { ArcBorrow::new(self.ptr) }
> }
> +
> + /// Compare whether two [`Arc`] pointers reference the same underlying object.
> + pub fn ptr_eq(this: &Self, other: &Self) -> bool {
> + core::ptr::eq(this.ptr.as_ptr(), other.ptr.as_ptr())
> + }
> }
>
> impl<T: 'static> ForeignOwnable for Arc<T> {
>
> base-commit: ac9a78681b921877518763ba0e89202254349d1b

2023-05-23 16:34:24

by Gary Guo

Subject: Re: [PATCH v1 2/2] rust: sync: implement `AsRef<T>` for `Arc<T>`

On Wed, 17 May 2023 20:08:14 +0000
Alice Ryhl <[email protected]> wrote:

> This trait lets you use `Arc<T>` in code that is generic over smart
> pointer types.
>
> The `AsRef` trait should be implemented on all smart pointers. The
> standard library also implements it on the ordinary `Arc`.
>
> Co-developed-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 274febe3bb06..9ec911e4a0c7 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -264,6 +264,12 @@ impl<T: ?Sized> Deref for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> AsRef<T> for Arc<T> {
> + fn as_ref(&self) -> &T {
> + self.deref()
> + }
> +}
> +
> impl<T: ?Sized> Clone for Arc<T> {
> fn clone(&self) -> Self {
> // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.

2023-05-23 16:34:40

by Gary Guo

Subject: Re: [PATCH v1 1/2] rust: sync: add `Arc::ptr_eq`

On Wed, 17 May 2023 20:08:13 +0000
Alice Ryhl <[email protected]> wrote:

> Add a method for comparing whether two `Arc` pointers reference the same
> underlying object.
>
> This comparison can already be done by getting a reference to the inner
> values and comparing whether the references have the same address.
> However, writing `Arc::ptr_eq(a, b)` is generally less error-prone than
> doing the same check on the references, since you might otherwise
> accidentally compare the two `&Arc<T>` references instead, which wont
> work because those are pointers to pointers to the inner value, when you
> just want to compare the pointers to the inner value.
>
> Also, this method might optimize better because getting a reference to
> the inner value involves offsetting the pointer, which this method does
> not need to do.
>
> Co-developed-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e6d206242465..274febe3bb06 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -221,6 +221,11 @@ impl<T: ?Sized> Arc<T> {
> // reference can be created.
> unsafe { ArcBorrow::new(self.ptr) }
> }
> +
> + /// Compare whether two [`Arc`] pointers reference the same underlying object.
> + pub fn ptr_eq(this: &Self, other: &Self) -> bool {
> + core::ptr::eq(this.ptr.as_ptr(), other.ptr.as_ptr())
> + }
> }
>
> impl<T: 'static> ForeignOwnable for Arc<T> {
>
> base-commit: ac9a78681b921877518763ba0e89202254349d1b

2023-05-25 14:27:37

by Benno Lossin

Subject: Re: [PATCH v1 1/2] rust: sync: add `Arc::ptr_eq`

On 5/17/23 22:08, Alice Ryhl wrote:
> Add a method for comparing whether two `Arc` pointers reference the same
> underlying object.
>
> This comparison can already be done by getting a reference to the inner
> values and comparing whether the references have the same address.
> However, writing `Arc::ptr_eq(a, b)` is generally less error-prone than
> doing the same check on the references, since you might otherwise
> accidentally compare the two `&Arc<T>` references instead, which wont
> work because those are pointers to pointers to the inner value, when you
> just want to compare the pointers to the inner value.
>
> Also, this method might optimize better because getting a reference to
> the inner value involves offsetting the pointer, which this method does
> not need to do.
>
> Co-developed-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>

Reviewed-by: Benno Lossin <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index e6d206242465..274febe3bb06 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -221,6 +221,11 @@ impl<T: ?Sized> Arc<T> {
> // reference can be created.
> unsafe { ArcBorrow::new(self.ptr) }
> }
> +
> + /// Compare whether two [`Arc`] pointers reference the same underlying object.
> + pub fn ptr_eq(this: &Self, other: &Self) -> bool {
> + core::ptr::eq(this.ptr.as_ptr(), other.ptr.as_ptr())
> + }
> }
>
> impl<T: 'static> ForeignOwnable for Arc<T> {
>
> base-commit: ac9a78681b921877518763ba0e89202254349d1b
> --
> 2.40.1.606.ga4b1b128d6-goog
>

--
Cheers,
Benno

2023-05-25 14:36:04

by Benno Lossin

Subject: Re: [PATCH v1 2/2] rust: sync: implement `AsRef<T>` for `Arc<T>`

On 5/17/23 22:08, Alice Ryhl wrote:
> This trait lets you use `Arc<T>` in code that is generic over smart
> pointer types.
>
> The `AsRef` trait should be implemented on all smart pointers. The
> standard library also implements it on the ordinary `Arc`.
>
> Co-developed-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Wedson Almeida Filho <[email protected]>
> Signed-off-by: Alice Ryhl <[email protected]>

Reviewed-by: Benno Lossin <[email protected]>

> ---
> rust/kernel/sync/arc.rs | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
> index 274febe3bb06..9ec911e4a0c7 100644
> --- a/rust/kernel/sync/arc.rs
> +++ b/rust/kernel/sync/arc.rs
> @@ -264,6 +264,12 @@ impl<T: ?Sized> Deref for Arc<T> {
> }
> }
>
> +impl<T: ?Sized> AsRef<T> for Arc<T> {
> + fn as_ref(&self) -> &T {
> + self.deref()
> + }
> +}
> +
> impl<T: ?Sized> Clone for Arc<T> {
> fn clone(&self) -> Self {
> // INVARIANT: C `refcount_inc` saturates the refcount, so it cannot overflow to zero.
> --
> 2.40.1.606.ga4b1b128d6-goog
>

--
Cheers,
Benno