diff mbox series

[v2,1/3] rust: block: simplify Result<()> in validate_block_size return

Message ID 20241118-simplify-result-v2-1-9d280ada516d@iiitd.ac.in (mailing list archive)
State New
Headers show
Series rust: simplify Result<()> uses | expand

Commit Message

Manas via B4 Relay Nov. 18, 2024, 1:12 p.m. UTC
From: Manas <manas18244@iiitd.ac.in>

`Result` is used in place of `Result<()>` because the default type
parameters are unit `()` and `Error` types, which are automatically
inferred. This patch keeps the usage consistent throughout codebase.

Signed-off-by: Manas <manas18244@iiitd.ac.in>
---
 rust/kernel/block/mq/gen_disk.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Miguel Ojeda Nov. 18, 2024, 2:08 p.m. UTC | #1
On Mon, Nov 18, 2024 at 2:12 PM Manas via B4 Relay
<devnull+manas18244.iiitd.ac.in@kernel.org> wrote:
>
> `Result` is used in place of `Result<()>` because the default type
> parameters are unit `()` and `Error` types, which are automatically
> inferred. This patch keeps the usage consistent throughout codebase.

The tags you had in v1 (Link, Suggested-by) seem to have been removed.

Nit: the usual style is to use the imperative tense when describing
the change that the patch performs, although that is not a hard rule,
e.g. you could say "Thus keep the usage consistent throughout the
codebase." in the last sentence.

> Signed-off-by: Manas <manas18244@iiitd.ac.in>

Same comment as in v1 about the "known identity".

(The notes above apply to the other patches too).

The change itself looks fine to me of course, so with those fixed,
please feel free to add in your next version:

Reviewed-by: Miguel Ojeda <ojeda@kernel.org>

Thanks!

Cheers,
Miguel
Manas Nov. 18, 2024, 2:22 p.m. UTC | #2
On 18.11.2024 15:08, Miguel Ojeda wrote:
>On Mon, Nov 18, 2024 at 2:12 PM Manas via B4 Relay
><devnull+manas18244.iiitd.ac.in@kernel.org> wrote:
>>
>> `Result` is used in place of `Result<()>` because the default type
>> parameters are unit `()` and `Error` types, which are automatically
>> inferred. This patch keeps the usage consistent throughout codebase.
>
>The tags you had in v1 (Link, Suggested-by) seem to have been removed.
>
>Nit: the usual style is to use the imperative tense when describing
>the change that the patch performs, although that is not a hard rule,
>e.g. you could say "Thus keep the usage consistent throughout the
>codebase." in the last sentence.
>
Will do.

>> Signed-off-by: Manas <manas18244@iiitd.ac.in>
>
>Same comment as in v1 about the "known identity".
>
Actually, "Manas" is my __official__ name.

>(The notes above apply to the other patches too).
>
>The change itself looks fine to me of course, so with those fixed,
>please feel free to add in your next version:
>
>Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
>
Adding tags in v3.
Miguel Ojeda Nov. 18, 2024, 4:29 p.m. UTC | #3
On Mon, Nov 18, 2024 at 3:22 PM Manas <manas18244@iiitd.ac.in> wrote:
>
> Actually, "Manas" is my __official__ name.

I didn't say otherwise (nor I meant to offend, I apologize if that is the case).

I worded my comment that way because a "known identity" does not need
to be an official/legal/birth name, e.g. some contributors prefer to
use a nickname, and that is perfectly fine too.

Cheers,
Miguel
Manas Nov. 18, 2024, 4:51 p.m. UTC | #4
On 18.11.2024 17:29, Miguel Ojeda wrote:
>On Mon, Nov 18, 2024 at 3:22 PM Manas <manas18244@iiitd.ac.in> wrote:
>>
>> Actually, "Manas" is my __official__ name.
>
>I didn't say otherwise (nor I meant to offend, I apologize if that is the case).
>
I didn't take an offense at all. :) No worries.

>I worded my comment that way because a "known identity" does not need
>to be an official/legal/birth name, e.g. some contributors prefer to
>use a nickname, and that is perfectly fine too.
>
I am in private talks with Andrew about this. I will CC you.
diff mbox series

Patch

diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 708125dce96a934f32caab44d5e6cff14c4321a9..798c4ae0bdedd58221b5851a630c0e1052e0face 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -45,7 +45,7 @@  pub fn rotational(mut self, rotational: bool) -> Self {
 
     /// Validate block size by verifying that it is between 512 and `PAGE_SIZE`,
     /// and that it is a power of two.
-    fn validate_block_size(size: u32) -> Result<()> {
+    fn validate_block_size(size: u32) -> Result {
         if !(512..=bindings::PAGE_SIZE as u32).contains(&size) || !size.is_power_of_two() {
             Err(error::code::EINVAL)
         } else {