Also fix a minor typo in one of the comments
Suggested-by: Benno Lossin <[email protected]>
Link: https://lore.kernel.org/rust-for-linux/[email protected]/
Link: https://github.com/Rust-for-Linux/linux/issues/1001
Signed-off-by: Ben Gooding <[email protected]>
---
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
On 5/7/23 17:22, Ben Gooding wrote:
> Also fix a minor typo in one of the comments
>
> Suggested-by: Benno Lossin <[email protected]>
> Link: https://lore.kernel.org/rust-for-linux/[email protected]/
> Link: https://github.com/Rust-for-Linux/linux/issues/1001
> Signed-off-by: Ben Gooding <[email protected]>
> ---
> -/// 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
Suggested-by: Alice Ryhl <[email protected]>
Signed-off-by: Ben Gooding <[email protected]>
---
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
Hi Ben,
On Sun, May 7, 2023 at 6:27 PM Ben Gooding <[email protected]> wrote:
>
> Suggested-by: Alice Ryhl <[email protected]>
> Signed-off-by: Ben Gooding <[email protected]>
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
On Sun, May 7, 2023 at 5:23 PM Ben Gooding <[email protected]> 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
Add missing intra-doc links to the Backend trait to make navigating the
documentation easier.
Suggested-by: Benno Lossin <[email protected]>
Link: https://lore.kernel.org/rust-for-linux/[email protected]/
Link: https://github.com/Rust-for-Linux/linux/issues/1001
Signed-off-by: Ben Gooding <[email protected]>
---
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
Hi Miguel,
On 08/05/2023 21:37, Miguel Ojeda wrote:
> Hi Ben,
>
> On Sun, May 7, 2023 at 6:27 PM Ben Gooding <[email protected]> wrote:
>> Suggested-by: Alice Ryhl <[email protected]>
>> Signed-off-by: Ben Gooding <[email protected]>
> 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
Miguel Ojeda <[email protected]> writes:
> On Sun, May 7, 2023 at 5:23 PM Ben Gooding <[email protected]> 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
On Tue, May 9, 2023 at 10:24 PM Ben Gooding <[email protected]> wrote:
>
> Add missing intra-doc links to the Backend trait to make navigating the
> documentation easier.
>
> Suggested-by: Benno Lossin <[email protected]>
> Link: https://lore.kernel.org/rust-for-linux/[email protected]/
> Link: https://github.com/Rust-for-Linux/linux/issues/1001
> Signed-off-by: Ben Gooding <[email protected]>
Applied to `rust-next` -- thanks!
Cheers,
Miguel