[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CANiq72n27gF0e3r_pZ_6ZD7EBhwCLghFkQtLwfApEGCQo6gcnQ@mail.gmail.com>
Date: Sun, 11 Jan 2026 18:57:24 +0100
From: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To: Gladyshev Ilya <foxido@...ido.dev>
Cc: Gary Guo <gary@...yguo.net>, "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>,
Benno Lossin <lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>, Tamir Duberstein <tamird@...il.com>, Armin Wolf <W_Armin@....de>,
platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: Re: [PATCH 2/3] rust: implement wrapper for acpi_object
On Fri, Jan 9, 2026 at 11:57 AM Gladyshev Ilya <foxido@...ido.dev> wrote:
>
> If I understand correctly, acpi_object is something untrusted in general
> and broken hardware can provide arbitrary _type value. So ergonomics of
> enum solution would be kinda strange:
>
> ```
> type_id() -> Result<Enum> // Result because it can be invalid
>
> // ...
>
> // '?' here makes it look like non-trivial operation
> if x.type_id()? == Enum::Buffer
> ```
Yeah, I was mentioning the `bindgen` bit because returning
`Result<Enum>` is essentially what we asked for the generated code,
i.e. we generate a panicking `From` impl, a fallible `TryFrom` impl
and an unsafe conversion method too.
Whether that may add overhead or not, it depends, of course. Sometimes
the result (i.e. the enum) may need to be used later on in several
places, and knowing statically that the value is in-bounds means there
is no need to recheck anymore for that reason.
That can actually mean reducing overhead (e.g. if checks exist in
several APIs, possibly defensively) or at least reduce the mistakes
that may be made tracking whether the value is valid or not.
But, of course, if the value is not used for anything on the Rust
side, and is just passed along, and the consumer of the value in the C
side do not have UB if it is out of range, and you generally don't
expect that to change, then checking validity upfront may indeed not
add much value like you say. From what you say (I don't know), it
seems that is the case and thus a newtype may be simpler and best.
But if those conditions change, e.g. you later start to want to
manipulate the value on the Rust side for any reason, then it will
likely be a different story. In such a case, then we shouldn't be
scared of the ergonomics of conversions, i.e. it is fine and expected
to have to do that, which is why we want to generate all that
boilerplate in `bindgen` eventually.
Or perhaps you may need both options, i.e. the newtype, and the other
one, depending on what you are doing with the value.
Regarding `ACPI_TYPE_INVALID`, yeah, it would depend on what is used
for. If nobody should be calling a given API with such a value, then
that API should return `Err`. But if it gets used as a useful marker
value somewhere, then it may need to be a valid value and thus return
`Ok`. If both cases are true, then perhaps again two ways may be
needed to model those.
Cheers,
Miguel
Powered by blists - more mailing lists