Re: [PATCH RESEND] block, rust: simplify validate_block_size() function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03.09.24 21:47, Dmitry Torokhov wrote:
> On Tue, Sep 03, 2024 at 09:30:53PM +0200, Miguel Ojeda wrote:
>> On Tue, Sep 3, 2024 at 9:00 PM Dmitry Torokhov
>> <dmitry.torokhov@xxxxxxxxx> wrote:
>>> able to parse the things quickly and not constantly think if my Rust is
>>> idiomatic enough or I could write the code in a more idiomatic way with
>>> something brand new that just got off the nightly list and moved into
>>> stable.
>>
>> If a feature is in the minimum support version we have for Rust in the
>> kernel, and it improves the way we write code, then we should consider
>> taking advantage of it.
>>
>> Now, that particular function call would have compiled since Rust 1.35
>> and ranges were already a concept back in Rust 1.0. So I am not sure
>> why you mention recently stabilized features here.
> 
> I was talking in general, not about this exact case, sorry if I was
> unclear. But in general I personally have this issue with Rust and to
> extent also with C++ where I constantly wonder if my code is "idiomatic
> enough" or if it looks obsolete because it is "only" C++14 and not 17
> or 20.
> 
> With C usually have no such concerns which allows me to concentrate on
> different things.

I personally don't worry about whether my code is idiomatic. I just code
and when others give suggestions that sound good, I apply them. There is
nobody requiring all Rust code to be as idiomatic as possible. In the
review process you might be nudged to write in a more idiomatic style,
but that should also be true for C. The kernel is constantly adding new
ways of doing things and while the language itself doesn't evolve, the
codebase surely does.

>> For this particular case, I don't think it matters too much, and I can
>> see arguments both ways (and we could introduce other ways to avoid
>> the reference or swap the order, e.g. `n.within(a..b)`).
>>
>>> This is a private function and an implementation detail. Why does it
>>> need to be exposed in documentation at all?
>>
>> That is a different question -- but even if it should be a private
>> function, it does not mean documentation should be removed (even if
>> currently we do not require documentation for private items).
> 
> I think exposing documentation for private function that can change at
> any time and is not callable from outside has little value. That does
> not mean that comments annotating such function have no value. But they
> need to be taken together with the function code, and in this case
> Alexey's concern about comments like "this increments that by 1"
> becomes quite valid.

For the private `validate_block_size` function, I can see your argument.
However, I don't think that the function will change any time soon, so I
don't think it's a huge issue.

---
Cheers,
Benno






[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux