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

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

[Alice Ryhl]

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 <walmeida@microsoft.com>
Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 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

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

[Alice Ryhl]

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 <walmeida@microsoft.com>
Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 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

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

[Martin Rodriguez Reboredo]

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 <yakoyoku@gmail.com>

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

[Andreas Hindborg]

Alice Ryhl <aliceryhl@google.com> 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 <walmeida@microsoft.com>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>

> ---
>  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.

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

[Gary Guo]

On Wed, 17 May 2023 20:08:14 +0000
Alice Ryhl <aliceryhl@google.com> 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 <walmeida@microsoft.com>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  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.

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

[Benno Lossin]

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 <walmeida@microsoft.com>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

> ---
>   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

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

[Martin Rodriguez Reboredo]

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 <yakoyoku@gmail.com>

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

[Andreas Hindborg]

Alice Ryhl <aliceryhl@google.com> 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 <walmeida@microsoft.com>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Andreas Hindborg <a.hindborg@samsung.com>

> ---
>  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

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

[Gary Guo]

On Wed, 17 May 2023 20:08:13 +0000
Alice Ryhl <aliceryhl@google.com> 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 <walmeida@microsoft.com>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  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

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

[Benno Lossin]

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 <walmeida@microsoft.com>
> Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

> ---
>   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