Re: [PATCH] docs: rust: clean up empty `\\\` lines and improve rustdoc formatting

From: Benno Lossin
Date: Mon Sep 09 2024 - 15:22:14 EST


Hi,

I see that this is your first kernel contribution, welcome!
I have left a couple comments below; before you send a new version,
please wait a couple days for other people to also leave their feedback.
You then create a new version (add `-v2` to `git format-patch`) and send
it to the list. You can put a changelog underneath the `---`, it will
not be included int the commit message, but for people reading the mail
it is rather helpful.

On 09.09.24 18:17, hridesh wrote:
> Remove unnecessary empty `\\\` lines in the rust docs. Also add linebreaks

You wrote backslashes here, but it should be forward slashes instead.
Please also fix it in the title.

I don't know if the commit title should start with `docs`, maybe we want
to do `rust: docs` when changing rustdocs? (This is a question to the
other Rust reviewers)

I think the title doesn't need to mention the exact cleanup, just
something along the lines "clean up docs" should suffice.

> in kernel::block::mq::Request to fix formatting
>
> Suggested-by: Miguel Ojeda <ojeda@xxxxxxxxxx>
> Link: https://github.com/Rust-for-Linux/linux/issues/1109

The issue also mentions that you should implement a `checkpatch.pl`
check in an additional patch:

> Clean up consecutive empty `///` lines and implement a checkpatch.pl
> check for it. These should be two different patches.

Please include that patch in your series.

> Signed-off-by: hridesh <hridesh699@xxxxxxxxx>
> ---
> rust/kernel/block/mq/request.rs | 7 +++----
> rust/kernel/rbtree.rs | 1 -
> 2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index a0e22827f3f4..3ab2917c9d25 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -22,15 +22,14 @@
> ///
> /// There are four states for a request that the Rust bindings care about:
> ///
> -/// A) Request is owned by block layer (refcount 0)
> +/// A) Request is owned by block layer (refcount 0)\

Instead of adding these backslashes, I personally would prefer if we
make this a normal markdown list using `1.`, `2.` etc.
Of course only if Andreas is OK with that though.

---
Cheers,
Benno

> /// B) Request is owned by driver but with zero `ARef`s in existence
> -/// (refcount 1)
> +/// (refcount 1)\
> /// C) Request is owned by driver with exactly one `ARef` in existence
> -/// (refcount 2)
> +/// (refcount 2)\
> /// D) Request is owned by driver with more than one `ARef` in existence
> /// (refcount > 2)
> ///
> -///
> /// We need to track A and B to ensure we fail tag to request conversions for
> /// requests that are not owned by the driver.
> ///
> diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
> index 25eb36fd1cdc..006f6e03aba5 100644
> --- a/rust/kernel/rbtree.rs
> +++ b/rust/kernel/rbtree.rs
> @@ -1031,7 +1031,6 @@ fn next(&mut self) -> Option<Self::Item> {
>
> /// A memory reservation for a red-black tree node.
> ///
> -///
> /// It contains the memory needed to hold a node that can be inserted into a red-black tree. One
> /// can be obtained by directly allocating it ([`RBTreeNodeReservation::new`]).
> pub struct RBTreeNodeReservation<K, V> {
> --
> 2.46.0
>