[PATCH] rust: lock: Add intra-doc links to the Backend trait

Ben Gooding posted 1 patch 2 years, 9 months ago
There is a newer version of this series
rust/kernel/sync/lock.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] rust: lock: Add intra-doc links to the Backend trait
Posted by Ben Gooding 2 years, 9 months ago
Also fix a minor typo in one of the comments

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/rust-for-linux/94625fe6-b87a-a8f0-5b2a-a8152d5f7436@proton.me/
Link: https://github.com/Rust-for-Linux/linux/issues/1001
Signed-off-by: Ben Gooding <ben.gooding.dev@gmail.com>
---
 rust/kernel/sync/lock.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index a2216325632d..95466201dab7 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -72,7 +72,7 @@ pub unsafe trait Backend {
 
 /// A mutual exclusion primitive.
 ///
-/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock backend
+/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock [backend](Backend)
 /// specified as the generic parameter `B`.
 #[pin_data]
 pub struct Lock<T: ?Sized, B: Backend> {
@@ -90,7 +90,7 @@ pub struct Lock<T: ?Sized, B: Backend> {
     pub(crate) data: UnsafeCell<T>,
 }
 
-// SAFETY: `Lock` can be transferred across thread boundaries iff the data it protects can.
+// SAFETY: `Lock` can be transferred across thread boundaries if the data it protects can.
 unsafe impl<T: ?Sized + Send, B: Backend> Send for Lock<T, B> {}
 
 // SAFETY: `Lock` serialises the interior mutability it provides, so it is `Sync` as long as the
@@ -126,7 +126,7 @@ impl<T: ?Sized, B: Backend> Lock<T, B> {
 
 /// A lock guard.
 ///
-/// Allows mutual exclusion primitives that implement the `Backend` trait to automatically unlock
+/// Allows mutual exclusion primitives that implement the [`Backend`] trait to automatically unlock
 /// when a guard goes out of scope. It also provides a safe and convenient way to access the data
 /// protected by the lock.
 #[must_use = "the lock unlocks immediately when the guard is unused"]
-- 
2.34.1
Re: [PATCH] rust: lock: Add intra-doc links to the Backend trait
Posted by Miguel Ojeda 2 years, 9 months ago
On Sun, May 7, 2023 at 5:23 PM Ben Gooding <ben.gooding.dev@gmail.com> wrote:
>
> Also fix a minor typo in one of the comments

"iff" is not a typo. Even if it were, it is best to avoid mixing
different types of changes in commits, to keep them as small as
possible (though sometimes there are exceptions).

> -/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock backend
> +/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock [backend](Backend)

What about simply:

    [`Backend`]

? (assuming it works).

Cheers,
Miguel
Re: [PATCH] rust: lock: Add intra-doc links to the Backend trait
Posted by Andreas Hindborg 2 years, 9 months ago
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:

> On Sun, May 7, 2023 at 5:23 PM Ben Gooding <ben.gooding.dev@gmail.com> wrote:
>>
>> Also fix a minor typo in one of the comments
>
> "iff" is not a typo. Even if it were, it is best to avoid mixing
> different types of changes in commits, to keep them as small as
> possible (though sometimes there are exceptions).

We should change the wording to "if and only if" to avoid confusion.

BR Andreas
[PATCH v2] rust: lock: Add intra-doc links to the Backend trait
Posted by Ben Gooding 2 years, 9 months ago
Add missing intra-doc links to the Backend trait to make navigating the
documentation easier.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/rust-for-linux/94625fe6-b87a-a8f0-5b2a-a8152d5f7436@proton.me/
Link: https://github.com/Rust-for-Linux/linux/issues/1001
Signed-off-by: Ben Gooding <ben.gooding.dev@gmail.com>
---
 rust/kernel/sync/lock.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index a2216325632d..05ac8107ff3c 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -72,8 +72,8 @@ pub unsafe trait Backend {
 
 /// A mutual exclusion primitive.
 ///
-/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock backend
-/// specified as the generic parameter `B`.
+/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock
+/// [`Backend`] specified as the generic parameter `B`.
 #[pin_data]
 pub struct Lock<T: ?Sized, B: Backend> {
     /// The kernel lock object.
@@ -126,7 +126,7 @@ impl<T: ?Sized, B: Backend> Lock<T, B> {
 
 /// A lock guard.
 ///
-/// Allows mutual exclusion primitives that implement the `Backend` trait to automatically unlock
+/// Allows mutual exclusion primitives that implement the [`Backend`] trait to automatically unlock
 /// when a guard goes out of scope. It also provides a safe and convenient way to access the data
 /// protected by the lock.
 #[must_use = "the lock unlocks immediately when the guard is unused"]
-- 
2.34.1
Re: [PATCH v2] rust: lock: Add intra-doc links to the Backend trait
Posted by Miguel Ojeda 2 years, 6 months ago
On Tue, May 9, 2023 at 10:24 PM Ben Gooding <ben.gooding.dev@gmail.com> wrote:
>
> Add missing intra-doc links to the Backend trait to make navigating the
> documentation easier.
>
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Link: https://lore.kernel.org/rust-for-linux/94625fe6-b87a-a8f0-5b2a-a8152d5f7436@proton.me/
> Link: https://github.com/Rust-for-Linux/linux/issues/1001
> Signed-off-by: Ben Gooding <ben.gooding.dev@gmail.com>

Applied to `rust-next` -- thanks!

Cheers,
Miguel
Re: [PATCH] rust: lock: Add intra-doc links to the Backend trait
Posted by Alice Ryhl 2 years, 9 months ago
On 5/7/23 17:22, Ben Gooding wrote:
> Also fix a minor typo in one of the comments
> 
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Link: https://lore.kernel.org/rust-for-linux/94625fe6-b87a-a8f0-5b2a-a8152d5f7436@proton.me/
> Link: https://github.com/Rust-for-Linux/linux/issues/1001
> Signed-off-by: Ben Gooding <ben.gooding.dev@gmail.com>
> ---
> -/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock backend
> +/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock [backend](Backend)

This line is too long. Please reflow at 100 characters.

You can also consider this option:

/// Exposes one of the kernel locking primitives. Which one is exposed 
depends on the lock [backend]
/// specified as the generic parameter `B`.
///
/// [backend]: Backend
[PATCH] rust: lock: Reflow long documentation line
Posted by Ben Gooding 2 years, 9 months ago
Suggested-by: Alice Ryhl <alice@ryhl.io>
Signed-off-by: Ben Gooding <ben.gooding.dev@gmail.com>
---
 rust/kernel/sync/lock.rs | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 95466201dab7..04413f6f145a 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -72,8 +72,10 @@ pub unsafe trait Backend {
 
 /// A mutual exclusion primitive.
 ///
-/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock [backend](Backend)
-/// specified as the generic parameter `B`.
+/// Exposes one of the kernel locking primitives. Which one is exposed depends on the lock
+/// [backend] specified as the generic parameter `B`.
+/// 
+/// [backend]: Backend
 #[pin_data]
 pub struct Lock<T: ?Sized, B: Backend> {
     /// The kernel lock object.
-- 
2.34.1
Re: [PATCH] rust: lock: Reflow long documentation line
Posted by Miguel Ojeda 2 years, 9 months ago
Hi Ben,

On Sun, May 7, 2023 at 6:27 PM Ben Gooding <ben.gooding.dev@gmail.com> wrote:
>
> Suggested-by: Alice Ryhl <alice@ryhl.io>
> Signed-off-by: Ben Gooding <ben.gooding.dev@gmail.com>

Thanks for the patch! Several notes:

  - Missing commit message -- in general, please check your patches
with `scripts/checkpatch.pl` and please read
https://docs.kernel.org/process/submitting-patches.html.

  - This patch goes on top of the previous one you sent but, in the
kernel workflow, what you are expected to do is send a v2 of your
patch series instead. You can use `-v2` in `git format-patch` for
that.

  - This patch is not just reflowing as the title implies, but it also
changes the style of the link -- is there a reason for that? If yes,
this should be explained.

Cheers,
Miguel
Re: [PATCH] rust: lock: Reflow long documentation line
Posted by Ben Gooding 2 years, 9 months ago
Hi Miguel,

On 08/05/2023 21:37, Miguel Ojeda wrote:
> Hi Ben,
>
> On Sun, May 7, 2023 at 6:27 PM Ben Gooding <ben.gooding.dev@gmail.com> wrote:
>> Suggested-by: Alice Ryhl <alice@ryhl.io>
>> Signed-off-by: Ben Gooding <ben.gooding.dev@gmail.com>
> Thanks for the patch! Several notes:
>
>    - Missing commit message -- in general, please check your patches
> with `scripts/checkpatch.pl` and please read
> https://docs.kernel.org/process/submitting-patches.html.
>
>    - This patch goes on top of the previous one you sent but, in the
> kernel workflow, what you are expected to do is send a v2 of your
> patch series instead. You can use `-v2` in `git format-patch` for
> that.
>
>    - This patch is not just reflowing as the title implies, but it also
> changes the style of the link -- is there a reason for that? If yes,
> this should be explained.
>
> Cheers,
> Miguel

Thank you very much for your feedback, this is really helpful as I'm 
learning the process.

I've submitted a proper v2 of my patch based on your feedback which is 
hopefully much more like what you would expect - please let me know.

Many thanks,

Ben