ACPI Object is represented via union on C-side. On Rust side, this union
is transparently wrapped for each ACPI Type, with individual methods and
Defer implementation to represented type (integer, string, buffer, etc).
Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
---
rust/kernel/acpi.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 91 insertions(+)
diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
index 9b8efa623130..ac1a9f305f6c 100644
--- a/rust/kernel/acpi.rs
+++ b/rust/kernel/acpi.rs
@@ -2,6 +2,8 @@
//! Advanced Configuration and Power Interface abstractions.
+use core::ops::Deref;
+
use crate::{
bindings,
device_id::{RawDeviceId, RawDeviceIdIndex},
@@ -63,3 +65,92 @@ macro_rules! acpi_device_table {
$crate::module_device_table!("acpi", $module_table_name, $table_name);
};
}
+
+/// An ACPI object.
+///
+/// This structure represents the Rust abstraction for a C [`struct acpi_object`].
+/// You probably want to convert it into actual object type (e.g [`AcpiBuffer`]).
+///
+/// # Example
+/// ```
+/// # use kernel::prelude::*;
+/// use kernel::acpi::{AcpiObject, AcpiBuffer};
+///
+/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
+/// let buf: &AcpiBuffer = obj.try_into()?;
+///
+/// Ok(buf[0])
+/// }
+/// ```
+///
+/// [`struct acpi_object`]: srctree/include/acpi/actypes.h
+#[repr(transparent)]
+pub struct AcpiObject(bindings::acpi_object);
+
+impl AcpiObject {
+ /// Returns object type id (see [`actypes.h`](srctree/include/acpi/actypes.h)).
+ pub fn type_id(&self) -> u32 {
+ // SAFETY: `type` field is valid in all union variants.
+ unsafe { self.0.type_ }
+ }
+}
+
+/// Generate wrapper type for AcpiObject subtype.
+///
+/// For given subtype implements
+/// - `#[repr(transparent)]` type wrapper,
+/// - `TryFrom<&AcpiObject> for &SubType` trait,
+/// - unsafe from_unchecked() for 'trusted' conversion.
+macro_rules! acpi_object_subtype {
+ ($subtype_name:ident <- ($acpi_type:ident, $field_name:ident, $union_type:ty)) => {
+ /// Wraps `acpi_object` subtype.
+ #[repr(transparent)]
+ pub struct $subtype_name($union_type);
+
+ impl<'a> TryFrom<&'a AcpiObject> for &'a $subtype_name {
+ type Error = Error;
+
+ fn try_from(value: &'a AcpiObject) -> core::result::Result<Self, Self::Error> {
+ if (value.type_id() != $subtype_name::ACPI_TYPE) {
+ return Err(EINVAL);
+ }
+
+ // SAFETY: Requested cast is valid because we validated type_id
+ Ok(unsafe { $subtype_name::from_unchecked(&value) })
+ }
+ }
+
+ impl $subtype_name {
+ /// Int value, representing this ACPI type (see [`acpitypes.h`]).
+ ///
+ /// [`acpitypes.h`]: srctree/include/linux/acpitypes.h
+ pub const ACPI_TYPE: u32 = bindings::$acpi_type;
+
+ /// Converts opaque AcpiObject reference into exact ACPI type reference.
+ ///
+ /// # Safety
+ ///
+ /// - Requested cast should be valid (value.type_id() is `Self::ACPI_TYPE`).
+ pub unsafe fn from_unchecked(value: &AcpiObject) -> &Self {
+ // SAFETY:
+ // - $field_name is currently active union's field due to external safety contract,
+ // - Transmuting to `repr(transparent)` wrapper is safe.
+ unsafe {
+ ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
+ }
+ }
+ }
+ };
+}
+
+acpi_object_subtype!(AcpiBuffer
+ <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
+
+impl Deref for AcpiBuffer {
+ type Target = [u8];
+
+ fn deref(&self) -> &Self::Target {
+ // SAFETY: (pointer, length) indeed represents byte slice.
+ unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
+ }
+}
--
2.52.0
On Wed, 7 Jan 2026 23:35:32 +0300
Gladyshev Ilya <foxido@foxido.dev> wrote:
> ACPI Object is represented via union on C-side. On Rust side, this union
> is transparently wrapped for each ACPI Type, with individual methods and
> Defer implementation to represented type (integer, string, buffer, etc).
>
> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
Hi Gladyshev,
I've checked the `acpi_object` implementation on the C side and it appears
that the buffer is not owned by the object (however managed externally,
could either be resting in ACPI tables directly or be allocated).
Therefore, you might want to carry a lifetime to represent the lifetime of
underlying buffers?
> ---
> rust/kernel/acpi.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 91 insertions(+)
>
> diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
> index 9b8efa623130..ac1a9f305f6c 100644
> --- a/rust/kernel/acpi.rs
> +++ b/rust/kernel/acpi.rs
> @@ -2,6 +2,8 @@
>
> //! Advanced Configuration and Power Interface abstractions.
>
> +use core::ops::Deref;
> +
> use crate::{
> bindings,
> device_id::{RawDeviceId, RawDeviceIdIndex},
> @@ -63,3 +65,92 @@ macro_rules! acpi_device_table {
> $crate::module_device_table!("acpi", $module_table_name, $table_name);
> };
> }
> +
> +/// An ACPI object.
> +///
> +/// This structure represents the Rust abstraction for a C [`struct acpi_object`].
> +/// You probably want to convert it into actual object type (e.g [`AcpiBuffer`]).
> +///
> +/// # Example
> +/// ```
> +/// # use kernel::prelude::*;
> +/// use kernel::acpi::{AcpiObject, AcpiBuffer};
> +///
> +/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
> +/// let buf: &AcpiBuffer = obj.try_into()?;
> +///
> +/// Ok(buf[0])
> +/// }
> +/// ```
> +///
> +/// [`struct acpi_object`]: srctree/include/acpi/actypes.h
> +#[repr(transparent)]
> +pub struct AcpiObject(bindings::acpi_object);
> +
> +impl AcpiObject {
> + /// Returns object type id (see [`actypes.h`](srctree/include/acpi/actypes.h)).
> + pub fn type_id(&self) -> u32 {
> + // SAFETY: `type` field is valid in all union variants.
> + unsafe { self.0.type_ }
> + }
This should probably be an enum instead of just integer.
> +}
> +
> +/// Generate wrapper type for AcpiObject subtype.
> +///
> +/// For given subtype implements
> +/// - `#[repr(transparent)]` type wrapper,
> +/// - `TryFrom<&AcpiObject> for &SubType` trait,
> +/// - unsafe from_unchecked() for 'trusted' conversion.
> +macro_rules! acpi_object_subtype {
> + ($subtype_name:ident <- ($acpi_type:ident, $field_name:ident, $union_type:ty)) => {
> + /// Wraps `acpi_object` subtype.
> + #[repr(transparent)]
> + pub struct $subtype_name($union_type);
Instead of wrapping the bindgen-generated subtypes, I would rather this to
be a transparent wrapper of `acpi_object`, with an invariant that the
specific union field is active.
This way you do not have to name the bindgen-generated names.
> +
> + impl<'a> TryFrom<&'a AcpiObject> for &'a $subtype_name {
> + type Error = Error;
> +
> + fn try_from(value: &'a AcpiObject) -> core::result::Result<Self, Self::Error> {
> + if (value.type_id() != $subtype_name::ACPI_TYPE) {
> + return Err(EINVAL);
> + }
> +
> + // SAFETY: Requested cast is valid because we validated type_id
> + Ok(unsafe { $subtype_name::from_unchecked(&value) })
> + }
> + }
It feels like this can be better implemented by having a sealed trait for
all possible ACPI object types?
> +
> + impl $subtype_name {
> + /// Int value, representing this ACPI type (see [`acpitypes.h`]).
> + ///
> + /// [`acpitypes.h`]: srctree/include/linux/acpitypes.h
> + pub const ACPI_TYPE: u32 = bindings::$acpi_type;
> +
> + /// Converts opaque AcpiObject reference into exact ACPI type reference.
> + ///
> + /// # Safety
> + ///
> + /// - Requested cast should be valid (value.type_id() is `Self::ACPI_TYPE`).
> + pub unsafe fn from_unchecked(value: &AcpiObject) -> &Self {
> + // SAFETY:
> + // - $field_name is currently active union's field due to external safety contract,
> + // - Transmuting to `repr(transparent)` wrapper is safe.
> + unsafe {
> + ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
> + }
> + }
> + }
> + };
> +}
> +
> +acpi_object_subtype!(AcpiBuffer
> + <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
> +
> +impl Deref for AcpiBuffer {
> + type Target = [u8];
> +
> + fn deref(&self) -> &Self::Target {
> + // SAFETY: (pointer, length) indeed represents byte slice.
> + unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
> + }
> +}
On 1/8/26 16:21, Gary Guo wrote:
> On Wed, 7 Jan 2026 23:35:32 +0300
> Gladyshev Ilya <foxido@foxido.dev> wrote:
>
>> ACPI Object is represented via union on C-side. On Rust side, this union
>> is transparently wrapped for each ACPI Type, with individual methods and
>> Defer implementation to represented type (integer, string, buffer, etc).
>>
>> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
>
>
> Hi Gladyshev,
>
> I've checked the `acpi_object` implementation on the C side and it appears
> that the buffer is not owned by the object (however managed externally,
> could either be resting in ACPI tables directly or be allocated).
Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object()
implementation, and it seems to me that the acpi_object's lifetime is
the same as its internal buffer. Overall, it is indeed managed
externally, but acpi_object and acpi_object::buffer->pointer live
together. I’m not an ACPI expert, though, so maybe I’m missing something.
Anyway, the current Rust setup seems fine for now:
0. AcpiObject validity is guaranteed by whoever constructed/passed it (C
side for WMI abstractions, for example)
1. You can only convert &AcpiObject to &AcpiSubType (reference to
reference), so AcpiSubType can't outlive AcpiObject
2. You can't steal the data pointer from &AcpiSubType either, because
the Deref impl is "logically safe" and only gives you a reference to the
inner data, which can't outlive AcpiSubType's reference -> can't outlive
AcpiObject.
So for now until AcpiObject lives _less_ than it's inner data,
everything is OK.
> Therefore, you might want to carry a lifetime to represent the lifetime of
> underlying buffers?
>> ---
>> rust/kernel/acpi.rs | 91 +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 91 insertions(+)
>>
>> diff --git a/rust/kernel/acpi.rs b/rust/kernel/acpi.rs
>> index 9b8efa623130..ac1a9f305f6c 100644
>> --- a/rust/kernel/acpi.rs
>> +++ b/rust/kernel/acpi.rs
>> @@ -2,6 +2,8 @@
>>
>> //! Advanced Configuration and Power Interface abstractions.
>>
>> +use core::ops::Deref;
>> +
>> use crate::{
>> bindings,
>> device_id::{RawDeviceId, RawDeviceIdIndex},
>> @@ -63,3 +65,92 @@ macro_rules! acpi_device_table {
>> $crate::module_device_table!("acpi", $module_table_name, $table_name);
>> };
>> }
>> +
>> +/// An ACPI object.
>> +///
>> +/// This structure represents the Rust abstraction for a C [`struct acpi_object`].
>> +/// You probably want to convert it into actual object type (e.g [`AcpiBuffer`]).
>> +///
>> +/// # Example
>> +/// ```
>> +/// # use kernel::prelude::*;
>> +/// use kernel::acpi::{AcpiObject, AcpiBuffer};
>> +///
>> +/// fn read_first_acpi_byte(obj: &AcpiObject) -> Result<u8> {
>> +/// let buf: &AcpiBuffer = obj.try_into()?;
>> +///
>> +/// Ok(buf[0])
>> +/// }
>> +/// ```
>> +///
>> +/// [`struct acpi_object`]: srctree/include/acpi/actypes.h
>> +#[repr(transparent)]
>> +pub struct AcpiObject(bindings::acpi_object);
>> +
>> +impl AcpiObject {
>> + /// Returns object type id (see [`actypes.h`](srctree/include/acpi/actypes.h)).
>> + pub fn type_id(&self) -> u32 {
>> + // SAFETY: `type` field is valid in all union variants.
>> + unsafe { self.0.type_ }
>> + }
>
> This should probably be an enum instead of just integer.
Using an enum would require keeping Rust's enum synced with the C side,
as well as implementing some simple but non-trivial checks that the
`type_` field is a valid enum value (and the valid range isn't
continuous). I think that keeping it as an integer is simpler and better
matches C side.
>> +}
>> +
>> +/// Generate wrapper type for AcpiObject subtype.
>> +///
>> +/// For given subtype implements
>> +/// - `#[repr(transparent)]` type wrapper,
>> +/// - `TryFrom<&AcpiObject> for &SubType` trait,
>> +/// - unsafe from_unchecked() for 'trusted' conversion.
>> +macro_rules! acpi_object_subtype {
>> + ($subtype_name:ident <- ($acpi_type:ident, $field_name:ident, $union_type:ty)) => {
>> + /// Wraps `acpi_object` subtype.
>> + #[repr(transparent)]
>> + pub struct $subtype_name($union_type);
>
> Instead of wrapping the bindgen-generated subtypes, I would rather this to
> be a transparent wrapper of `acpi_object`, with an invariant that the
> specific union field is active.
>
> This way you do not have to name the bindgen-generated names.
Sounds reasonable, thanks!
>> +
>> + impl<'a> TryFrom<&'a AcpiObject> for &'a $subtype_name {
>> + type Error = Error;
>> +
>> + fn try_from(value: &'a AcpiObject) -> core::result::Result<Self, Self::Error> {
>> + if (value.type_id() != $subtype_name::ACPI_TYPE) {
>> + return Err(EINVAL);
>> + }
>> +
>> + // SAFETY: Requested cast is valid because we validated type_id
>> + Ok(unsafe { $subtype_name::from_unchecked(&value) })
>> + }
>> + }
>
> It feels like this can be better implemented by having a sealed trait for
> all possible ACPI object types?
I don’t see any particular benefit except moving the `impl TryFrom` out
of macro-generated code, but you’re probably right -- I’ll try
implementing it this way.
(will rustdoc show TryFrom on the AcpiSubType's page if it's implemented
via sealed trait?)
>> +
>> + impl $subtype_name {
>> + /// Int value, representing this ACPI type (see [`acpitypes.h`]).
>> + ///
>> + /// [`acpitypes.h`]: srctree/include/linux/acpitypes.h
>> + pub const ACPI_TYPE: u32 = bindings::$acpi_type;
>> +
>> + /// Converts opaque AcpiObject reference into exact ACPI type reference.
>> + ///
>> + /// # Safety
>> + ///
>> + /// - Requested cast should be valid (value.type_id() is `Self::ACPI_TYPE`).
>> + pub unsafe fn from_unchecked(value: &AcpiObject) -> &Self {
>> + // SAFETY:
>> + // - $field_name is currently active union's field due to external safety contract,
>> + // - Transmuting to `repr(transparent)` wrapper is safe.
>> + unsafe {
>> + ::core::mem::transmute::<&$union_type, &$subtype_name>(&value.0.$field_name)
>> + }
>> + }
>> + }
>> + };
>> +}
>> +
>> +acpi_object_subtype!(AcpiBuffer
>> + <- (ACPI_TYPE_BUFFER, buffer, bindings::acpi_object__bindgen_ty_3));
>> +
>> +impl Deref for AcpiBuffer {
>> + type Target = [u8];
>> +
>> + fn deref(&self) -> &Self::Target {
>> + // SAFETY: (pointer, length) indeed represents byte slice.
>> + unsafe { ::core::slice::from_raw_parts(self.0.pointer, self.0.length as usize) }
>> + }
>> +}
>
On Thu Jan 8, 2026 at 5:11 PM GMT, Gladyshev Ilya wrote:
> On 1/8/26 16:21, Gary Guo wrote:
>> On Wed, 7 Jan 2026 23:35:32 +0300
>> Gladyshev Ilya <foxido@foxido.dev> wrote:
>>
>>> ACPI Object is represented via union on C-side. On Rust side, this union
>>> is transparently wrapped for each ACPI Type, with individual methods and
>>> Defer implementation to represented type (integer, string, buffer, etc).
>>>
>>> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev>
>>
>>
>> Hi Gladyshev,
>>
>> I've checked the `acpi_object` implementation on the C side and it appears
>> that the buffer is not owned by the object (however managed externally,
>> could either be resting in ACPI tables directly or be allocated).
> Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object()
> implementation, and it seems to me that the acpi_object's lifetime is
> the same as its internal buffer.
No, it's not the same. acpi_object's lifetime needs to be shorter than
the internal buffer.
In Rust this is a typical case where you'd put lifetime on the struct
where you write
struct AcpiObject<'a> { ... }
> Overall, it is indeed managed
> externally, but acpi_object and acpi_object::buffer->pointer live
> together. I’m not an ACPI expert, though, so maybe I’m missing something.
>
> Anyway, the current Rust setup seems fine for now:
> 0. AcpiObject validity is guaranteed by whoever constructed/passed it (C
> side for WMI abstractions, for example)
When you construct a `AcpiObject`; it becomes incorrect: the constructed
`AcpiObject` now can outlive the internal buffer, which is broken.
Your code indeed works fine today, but the reason is that nobody can
construct a `AcpiObject`. They can only ever receive a reference, which
makes the issue disappear, as the lifetime gets "absorbed" by the
lifetime on the reference. In essense, whenever `&'a AcpiObject` appears
in your code, it's actually meant to be `&'a AcpiObject<'a>`.
If someone were to add a Rust-side constructor for `AcpiObject`, then
the lifetime becomes broken.
So it works today, but I think it gives the wrong impression to the user
that the buffer is managed by `AcpiObject`.
Best,
Gary
> 1. You can only convert &AcpiObject to &AcpiSubType (reference to
> reference), so AcpiSubType can't outlive AcpiObject
> 2. You can't steal the data pointer from &AcpiSubType either, because
> the Deref impl is "logically safe" and only gives you a reference to the
> inner data, which can't outlive AcpiSubType's reference -> can't outlive
> AcpiObject.
>
> So for now until AcpiObject lives _less_ than it's inner data,
> everything is OK.
>
>> Therefore, you might want to carry a lifetime to represent the lifetime of
>> underlying buffers?
On 1/8/26 23:06, Gary Guo wrote: > On Thu Jan 8, 2026 at 5:11 PM GMT, Gladyshev Ilya wrote: >> On 1/8/26 16:21, Gary Guo wrote: >>> On Wed, 7 Jan 2026 23:35:32 +0300 >>> Gladyshev Ilya <foxido@foxido.dev> wrote: >>> >>>> ACPI Object is represented via union on C-side. On Rust side, this union >>>> is transparently wrapped for each ACPI Type, with individual methods and >>>> Defer implementation to represented type (integer, string, buffer, etc). >>>> >>>> Signed-off-by: Gladyshev Ilya <foxido@foxido.dev> >>> >>> >>> Hi Gladyshev, >>> >>> I've checked the `acpi_object` implementation on the C side and it appears >>> that the buffer is not owned by the object (however managed externally, >>> could either be resting in ACPI tables directly or be allocated). >> Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object() >> implementation, and it seems to me that the acpi_object's lifetime is >> the same as its internal buffer. > > No, it's not the same. acpi_object's lifetime needs to be shorter than > the internal buffer. Okay, I agree than) Will fix in next revision
On Thu, Jan 8, 2026 at 6:11 PM Gladyshev Ilya <foxido@foxido.dev> wrote:
>
> Hm, I looked through ACPI_FREE() call sites and acpi_evaluate_object()
> implementation, and it seems to me that the acpi_object's lifetime is
> the same as its internal buffer. Overall, it is indeed managed
> externally, but acpi_object and acpi_object::buffer->pointer live
> together. I’m not an ACPI expert, though, so maybe I’m missing something.
>
> Anyway, the current Rust setup seems fine for now:
> 0. AcpiObject validity is guaranteed by whoever constructed/passed it (C
> side for WMI abstractions, for example)
> 1. You can only convert &AcpiObject to &AcpiSubType (reference to
> reference), so AcpiSubType can't outlive AcpiObject
> 2. You can't steal the data pointer from &AcpiSubType either, because
> the Deref impl is "logically safe" and only gives you a reference to the
> inner data, which can't outlive AcpiSubType's reference -> can't outlive
> AcpiObject.
>
> So for now until AcpiObject lives _less_ than it's inner data,
> everything is OK.
Assuming this is correct, this sort of analysis is typically nice to
keep documented as as an implementation comment (`//`) somewhere
(instead of just in the mailing list or the commit message) -- would
that make sense?
> Using an enum would require keeping Rust's enum synced with the C side,
> as well as implementing some simple but non-trivial checks that the
> `type_` field is a valid enum value (and the valid range isn't
> continuous). I think that keeping it as an integer is simpler and better
> matches C side.
If this refers to the `ACPI_TYPE_*` constants, there seems to be a
comment in the C docs that requires it to be kept in sync already with
elsewhere, so perhaps it could be reasonable to add one more place to
sync? (Though I don't see the mentioned arrays from a quick grep?)
* NOTE: Types must be kept in sync with the global acpi_ns_properties
* and acpi_ns_type_names arrays.
Ideally, these would be actual `enum`s on the C side and then
eventually we should be able to have `bindgen` generate the new kind
of `enum` that keeps this in sync and generates those checks for us,
see my suggestion at:
Support a new "enum variant" kind, similar to `rustified_enum`, that
provides a safe method to transform a C `enum` to Rust.
https://github.com/Rust-for-Linux/linux/issues/353
(But we can't do it just know, and even if it lands in `bindgen` we
will probably need to wait to bump the minimum etc.)
Cheers,
Miguel
On 1/8/26 21:46, Miguel Ojeda wrote:
>> Using an enum would require keeping Rust's enum synced with the C side,
>> as well as implementing some simple but non-trivial checks that the
>> `type_` field is a valid enum value (and the valid range isn't
>> continuous). I think that keeping it as an integer is simpler and better
>> matches C side.
>
> If this refers to the `ACPI_TYPE_*` constants, there seems to be a
> comment in the C docs that requires it to be kept in sync already with
> elsewhere, so perhaps it could be reasonable to add one more place to
> sync? (Though I don't see the mentioned arrays from a quick grep?)
>
> * NOTE: Types must be kept in sync with the global acpi_ns_properties
> * and acpi_ns_type_names arrays.
>
> [...snip about bindgen...]
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
```
And it's unclear should ACPI_TYPE_INVALID be Ok(INVALID) or Err...
Also users of AcpiObject generally doesn't care if received object has
valid, but unexpected type or invalid type, so this conversion int->enum
will introduce (small but) unneeded overhead.
That's why I preferred int-style approach, matching C side. Here is
alternative proposal from my side:
1. Enum with type values like you want
2. This Enum implements .id() method that returns underlying <int>
3. AcpiObject::type_id() still returns <int> value
4. There is method to convert int into enum (like TryFrom)
Probably you can 'newtype' <int> and implement TryFrom AcpiTypeId ->
Enum and write something like
```
match x.type_id().try_into()? /* or as_enum() or smth */ {
Enum::Integer => /* ... */,
Enum::String => /* ... */
}
```
---
Sorry for my chaotic explanations...
On Fri, Jan 9, 2026 at 11:57 AM Gladyshev Ilya <foxido@foxido.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
© 2016 - 2026 Red Hat, Inc.