linux-kernel - Re: [PATCH RFC 1/3] rust: add useful ops for u64

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D7W119MHCCWH.IS600FTIOV8O@nvidia.com>
Date: Wed, 19 Feb 2025 10:21:38 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Timur Tabi" <ttabi@...dia.com>, "Alexandre Courbot"
 <acourbot@...dia.com>, "daniel.almeida@...labora.com"
 <daniel.almeida@...labora.com>
Cc: "John Hubbard" <jhubbard@...dia.com>, "dri-devel@...ts.freedesktop.org"
 <dri-devel@...ts.freedesktop.org>, "rust-for-linux@...r.kernel.org"
 <rust-for-linux@...r.kernel.org>, "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>, "nouveau@...ts.freedesktop.org"
 <nouveau@...ts.freedesktop.org>, "dakr@...nel.org" <dakr@...nel.org>,
 "airlied@...il.com" <airlied@...il.com>, "Ben Skeggs" <bskeggs@...dia.com>
Subject: Re: [PATCH RFC 1/3] rust: add useful ops for u64

On Wed Feb 19, 2025 at 5:51 AM JST, Timur Tabi wrote:
> On Tue, 2025-02-18 at 22:16 +0900, Alexandre Courbot wrote:
>> > A proper struct with `high` and `low` might be more verbose, but
>> > it rules out this issue.
>> 
>> Mmm indeed, so we would have client code looking like:
>> 
>>   let SplitU64 { high, low } = some_u64.into_u32();
>> 
>> instead of 
>> 
>>   let (high, low) = some_u64.into_u32();
>> 
>> which is correct, and
>> 
>>   let (low, high) = some_u64.into_u32();
>> 
>> which is incorrect, but is likely to not be caught.
>
> I'm new to Rust, so let me see if I get this right.
>
> struct SplitU64 {
>   high: u32,
>   low: u32
> }
>
> So if you want to extract the upper 32 bits of a u64, you have to do this:
>
> let split = some_u64.into_u32s();
> let some_u32 = split.high;

More likely this would be something like:

  let SplitU64 { high: some_u32, .. } = some_u64;

Which is still a bit verbose, but a single-liner.

Actually. How about adding methods to this trait that return either
component?

  let some_u32 = some_u64.high_half();
  let another_u32 = some_u64.low_half();

These should be used most of the times, and using destructuring/tuple
would only be useful for a few select cases.

>
> as opposed to your original design:
>
> let (some_u32, _) = some_u64.into_u32s();
>
> Personally, I prefer the latter.  The other advantage is that into_u32s and
> from_u32s are reciprocal:
>
> assert_eq!(u64::from_u32s(u64::into_u32s(some_u64)), some_u64);
>
> (or something like that)

Yeah, having symmetry is definitely nice. OTOH there are no safeguards
against mixing up the order and the high and low components, so a
compromise will have to be made one way or the other. But if we also add
the methods I proposed above, that question should matter less.