Messages in this thread | | | From | Miguel Ojeda <> | Date | Tue, 27 Sep 2022 17:43:11 +0200 | Subject | Re: [PATCH v10 12/27] rust: add `kernel` crate |
| |
On Tue, Sep 27, 2022 at 5:22 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > This feels "odd" to me. Why not just use __kmalloc() instead of > krealloc()? I think that will get you the same kasan tracking, and > should be a tiny bit faster (1-2 less function calls). > > I guess it probably doesn't matter right now, just curious, and not a > big deal at all.
Yeah, nowadays I think a "C helper" could have been used instead.
> You'll be adding other error values here over time, right?
Indeed, I removed all the ones we didn't use in v8 to reduce it a bit more. Sorry for the confusion! :)
> What about functions that do have return functions of: > >= 0 number of bytes read/written/consumed/whatever > < 0 error code > > Would that use Result or Error as a type? Or is it best just to not try > to model that mess in Rust calls? :)
`Result`, i.e. the "number of bytes" part would go in the `Ok` variant and the "error code" in the `Err` variant.
The benefit is that then you have to handle them "separately", i.e. you cannot confuse the number of bytes for the error code by mistake, or vice versa.
> In the long run, using "raw" print macros like this is usually not the > thing to do. Drivers always have a device to reference the message to, > and other things like filesystems and subsystems have a prefix to use as > well. > > Hopefully not many will use these as-is and we can wrap them properly > later on.
Definitely, we will have e.g. the `dev_*!` ones:
https://github.com/Rust-for-Linux/linux/blob/fcad53ca9071c7bf6a412640a82e679bad6d1cd4/rust/kernel/device.rs#L479-L502
> Anyway, all looks sane to me, sorry for the noise: > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Thanks a lot for taking a look!
Cheers, Miguel
| |