[PATCH v3 1/2 RESEND] rust: kernel: clean up empty `///` lines

[PATCH v3 1/2 RESEND] rust: kernel: clean up empty `///` lines

[Hridesh MG]

Remove unnecessary empty `///` lines in the rust docs.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1109
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
Changelog:
v1->v2
- Fixed typo in commit title and description
- Removed backslashes in kernel::block::mq::Request
- Link to v1: https://lore.kernel.org/rust-for-linux/20240909161749.147076-1-hridesh699@gmail.com/

v2->v3
- Fixed From: tag in patch header
- Link to v2: https://lore.kernel.org/rust-for-linux/aa1b4059dfac001945745db02b6f6d9db2e5d1cb.1726072795.git.hridesh699@gmail.com/
---
 rust/kernel/block/mq/request.rs | 1 -
 rust/kernel/rbtree.rs           | 1 -
 2 files changed, 2 deletions(-)

diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index a0e22827f3f4..313334b1bf18 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -30,7 +30,6 @@
 /// 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.1

[PATCH v3 2/2 RESEND] checkpatch: warn on empty rust doc comments

[Hridesh MG]

Add a check to warn if there are consecutive empty `///` lines in rust
files.

Suggested-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://github.com/Rust-for-Linux/linux/issues/1109
Signed-off-by: Hridesh MG <hridesh699@gmail.com>
---
No changes in v2, this is a new patch
No changes in v3
---
 scripts/checkpatch.pl | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 39032224d504..c75bc3927bf6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3884,6 +3884,13 @@ sub process {
 			}
 		}
 
+# check for consecutive empty /// lines in .rs files
+		if ($realfile =~ /\.rs$/ &&
+		    $rawline =~ /^\+\s*\/\/\/$/ && $prevrawline =~ /^\+\s*\/\/\/$/) {
+			WARN("RUST_DOC_EMPTY",
+			     "avoid using consecutive empty rustdoc comments\n" . $herecurr);
+		}
+
 # check for adding lines without a newline.
 		if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) {
 			if (WARN("MISSING_EOF_NEWLINE",
-- 
2.46.1

Re: [PATCH v3 2/2 RESEND] checkpatch: warn on empty rust doc comments

[Trevor Gross]

On Sun, Sep 29, 2024 at 7:16 AM Hridesh MG <hridesh699@gmail.com> wrote:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 39032224d504..c75bc3927bf6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3884,6 +3884,13 @@ sub process {
>                         }
>                 }
>
> +# check for consecutive empty /// lines in .rs files
> +               if ($realfile =~ /\.rs$/ &&
> +                   $rawline =~ /^\+\s*\/\/\/$/ && $prevrawline =~ /^\+\s*\/\/\/$/) {
> +                       WARN("RUST_DOC_EMPTY",
> +                            "avoid using consecutive empty rustdoc comments\n" . $herecurr);
> +               }
> +

We got a request to add a rust-specific block in another patchset [1].
It would probably be good to coordinate here and have a patch adding
an empty block, to be used by both of these changes.

[1]: https://lore.kernel.org/rust-for-linux/a9112679-b251-4b98-b55e-e8aabf82ad46@proton.me/

Re: [PATCH v3 2/2 RESEND] checkpatch: warn on empty rust doc comments

[Patrick Miller]

[-- Attachment #1.1: Type: text/plain, Size: 1385 bytes --]






On Sunday, September 29th, 2024 at 6:15 PM, Trevor Gross <tmgross@umich.edu> wrote:

> 

> 

> On Sun, Sep 29, 2024 at 7:16 AM Hridesh MG hridesh699@gmail.com wrote:
> 

> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 39032224d504..c75bc3927bf6 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -3884,6 +3884,13 @@ sub process {
> > }
> > }
> > 

> > +# check for consecutive empty /// lines in .rs files
> > + if ($realfile =~ /\.rs$/ &&
> > + $rawline =~ /^\+\s*\/\/\/$/ && $prevrawline =~ /^\+\s*\/\/\/$/) {
> > + WARN("RUST_DOC_EMPTY",
> > + "avoid using consecutive empty rustdoc comments\n" . $herecurr);
> > + }
> > +
> 

> 

> We got a request to add a rust-specific block in another patchset [1].
> It would probably be good to coordinate here and have a patch adding
> an empty block, to be used by both of these changes.
> 

> [1]: https://lore.kernel.org/rust-for-linux/a9112679-b251-4b98-b55e-e8aabf82ad46@proton.me/

After the latest revision of my patch [1], I split the if statement so that
there is a parent check for rust files for future rust patch checks. So,
this would perfectly fit within that block.

Do you want me to add your code and credit you in my patch? 


[1]: https://lore.kernel.org/rust-for-linux/20240914181618.837227-2-paddymills@proton.me/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: [PATCH v3 2/2 RESEND] checkpatch: warn on empty rust doc comments

[Hridesh MG]

On Mon, Sep 30, 2024 at 8:11 AM Patrick Miller <paddymills@proton.me> wrote:
> After the latest revision of my patch [1], I split the if statement so that
> there is a parent check for rust files for future rust patch checks. So,
> this would perfectly fit within that block.
>
> Do you want me to add your code and credit you in my patch?
>
>
> [1]: https://lore.kernel.org/rust-for-linux/20240914181618.837227-2-paddymills@proton.me/

Please disregard my prior email, I had failed to CC everyone. Since
the change is small I'm okay with adding it as part of your patch,
please do add the Co-developed-by tag, thanks!

However, I was curious how conflicts like these are generally
resolved. For example, if there are two large patchsets which conflict
with each other, how does one ensure that they are compatible, and
even if they are, how do the maintainers ensure that they are applied
in the correct order?

Re: [PATCH v3 2/2 RESEND] checkpatch: warn on empty rust doc comments

[Miguel Ojeda]

On Mon, Sep 30, 2024 at 12:35 PM Hridesh MG <hridesh699@gmail.com> wrote:
>
> However, I was curious how conflicts like these are generally
> resolved. For example, if there are two large patchsets which conflict
> with each other, how does one ensure that they are compatible, and
> even if they are, how do the maintainers ensure that they are applied
> in the correct order?

The maintainers will usually figure it out (resolving the conflicts
when they apply them, applying them in the right order, etc.).
Otherwise, they can also ask for one of them to be resubmitted on top
of the other when it is too involved / subtle / large series.

If you are aware that you need a patch to be put on top of another,
then you can rebase it yourself of course. Just please make it clear
in the cover letter (or after the `---` part) what the patches apply
against.

Cheers,
Miguel

Re: [PATCH v3 2/2 RESEND] checkpatch: warn on empty rust doc comments

[Hridesh MG]

On Mon, Sep 30, 2024 at 4:14 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> The maintainers will usually figure it out (resolving the conflicts
> when they apply them, applying them in the right order, etc.).
> Otherwise, they can also ask for one of them to be resubmitted on top
> of the other when it is too involved / subtle / large series.
>
> If you are aware that you need a patch to be put on top of another,
> then you can rebase it yourself of course. Just please make it clear
> in the cover letter (or after the `---` part) what the patches apply
> against.
>
> Cheers,
> Miguel

That makes sense. Thank you for the clarification, Miguel.

Re: [PATCH v3 2/2 RESEND] checkpatch: warn on empty rust doc comments

[Patrick Miller]

[-- Attachment #1.1: Type: text/plain, Size: 1470 bytes --]






On Monday, September 30th, 2024 at 6:34 AM, Hridesh MG <hridesh699@gmail.com> wrote:

> 

> 

> On Mon, Sep 30, 2024 at 8:11 AM Patrick Miller paddymills@proton.me wrote:
> 

> > After the latest revision of my patch 1, I split the if statement so that
> > there is a parent check for rust files for future rust patch checks. So,
> > this would perfectly fit within that block.
> > 

> > Do you want me to add your code and credit you in my patch?
> 

> 

> Please disregard my prior email, I had failed to CC everyone. Since
> the change is small I'm okay with adding it as part of your patch,
> please do add the Co-developed-by tag, thanks!
> 

> However, I was curious how conflicts like these are generally
> resolved. For example, if there are two large patchsets which conflict
> with each other, how does one ensure that they are compatible, and
> even if they are, how do the maintainers ensure that they are applied
> in the correct order?

2 changes that I am making to your patch as I merge it with mine (I tested these)
  - Added a @fix option. I ran into this with my patch and was requested to
      add it by a checkpatch maintainer
  - Revised your $prevrawline regex to check against existing blank lines as
      well as added blank lines (made the leading + optional). Otherwise I
      think the checkpatch would not match against a blank doc comment line
      added after an existing blank doc line.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: [PATCH v3 1/2 RESEND] rust: kernel: clean up empty `///` lines

[Trevor Gross]

On Sun, Sep 29, 2024 at 7:17 AM Hridesh MG <hridesh699@gmail.com> wrote:
>
> Remove unnecessary empty `///` lines in the rust docs.
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1109
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>

Reviewed-by: Trevor Gross <tmgross@umich.edu>

Re: [PATCH v3 1/2 RESEND] rust: kernel: clean up empty `///` lines

[Alice Ryhl]

On Sun, Sep 29, 2024 at 1:15 PM Hridesh MG <hridesh699@gmail.com> wrote:
>
> Remove unnecessary empty `///` lines in the rust docs.
>
> Suggested-by: Miguel Ojeda <ojeda@kernel.org>
> Link: https://github.com/Rust-for-Linux/linux/issues/1109
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v3 1/2 RESEND] rust: kernel: clean up empty `///` lines

[Miguel Ojeda]

On Sun, Sep 29, 2024 at 1:15 PM Hridesh MG <hridesh699@gmail.com> wrote:
>
> Signed-off-by: Hridesh MG <hridesh699@gmail.com>

The patch looks good, thanks!

However, the kernel needs known identities to accept patches -- I know
you added "MG" since the first version when I asked, but is that your
full surname? i.e. it appears to be initials -- if they are not, then
I apologize in advance.

Cheers,
Miguel

Re: [PATCH v3 1/2 RESEND] rust: kernel: clean up empty `///` lines

[Hridesh MG]

On Mon, Sep 30, 2024 at 2:19 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> However, the kernel needs known identities to accept patches -- I know
> you added "MG" since the first version when I asked, but is that your
> full surname? i.e. it appears to be initials -- if they are not, then
> I apologize in advance.
>
> Cheers,
> Miguel

Ah, I think this is a cultural difference. The "MG" does indeed have
an expansion but it's only used for non-official purposes; my name is
stamped as "Hridesh MG" on all legal documents, so I suppose it counts
as a known identity.

Re: [PATCH v3 1/2 RESEND] rust: kernel: clean up empty `///` lines

[Miguel Ojeda]

On Mon, Sep 30, 2024 at 12:36 PM Hridesh MG <hridesh699@gmail.com> wrote:
>
> Ah, I think this is a cultural difference. The "MG" does indeed have
> an expansion but it's only used for non-official purposes; my name is
> stamped as "Hridesh MG" on all legal documents, so I suppose it counts
> as a known identity.

If that is your legal name, then it is fine -- thanks for clarifying!

Cheers,
Miguel