[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <284c09c9-c4ed-4838-a76b-dc90210b0be6@foxido.dev>
Date: Fri, 9 Jan 2026 14:31:08 +0300
From: Gladyshev Ilya <foxido@...ido.dev>
To: Kari Argillander <kari.argillander@...il.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>, 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 3/3] rust: add WMI abstractions
On 1/9/26 14:15, Kari Argillander wrote:
> pe 9.1.2026 klo 13.01 Gladyshev Ilya (foxido@...ido.dev) kirjoitti:
>>
>> On 1/8/26 23:48, Kari Argillander wrote:
>>> On Wed, 7 Jan 2026 at 22:56, Gladyshev Ilya <foxido@...ido.dev> wrote:
>>> <snip>
>>>
>>>> +impl DeviceId {
>>>> + /// Constructs new DeviceId from GUID string.
>>>> + pub const fn new(guid: &[u8; bindings::UUID_STRING_LEN as usize]) -> Self {
>>>> + // SAFETY: FFI type is valid to be zero-initialized.
>>>> + let mut inner: bindings::wmi_device_id = unsafe { MaybeUninit::zeroed().assume_init() };
>>>> +
>>>> + build_assert!(inner.guid_string.len() == bindings::UUID_STRING_LEN as usize + 1);
>>>> +
>>>> + // SAFETY: It's safe to copy UUID_STRING_LEN, because we validated lengths.
>>>> + // Also we leave last byte zeroed, so guid_string is valid C string.
>>>> + unsafe {
>>>> + ::core::ptr::copy_nonoverlapping(
>>>> + guid.as_ptr(),
>>>> + &raw mut inner.guid_string[0],
>>>> + bindings::UUID_STRING_LEN as usize,
>>>> + );
>>>> + }
>>>
>>> Just use while here so no unsafe is needed at all. Then probably patch
>>> 1/3 is not needed.
>>
>> Overall this operation is still unsafe because we are constructing C
>> string in FFI object. So for me avoiding `unsafe` via less readable
>> (imo) loop will just mask unsafe operation without any real benefits.
>
> It is not unsafe if you also use pin_init::zeroed()
> let mut inner: bindings::wmi_device_id = pin_init::zeroed();
>
> let mut i = 0usize;
> while i < bindings::UUID_STRING_LEN as usize {
> inner.guid_string[i] = guid[i];
> i += 1;
> }
>
> you can then also remove 'core::mem::MaybeUninit'
I was talking more about "constructing C string is still 'unsafe'
because you shouldn't miss the \0 byte". IMO unsafe but primitive memcpy
is more readable and alerting than while loop, but that's not something
I will insist on
I'll play around more with `guid: &CStr` API, probably I missed
something simple)
Powered by blists - more mailing lists