Re: [PATCH 1/5] gpu: nova-core: add Tlv.get_u64

[John Hubbard <jhubbard@nvidia.com>]

On 11/10/26 6:59 PM, Timur Tabi wrote:
> On Tue, 2026-11-10 at 16:20 -0800, John Hubbard wrote:
>> On 11/10/26 3:10 PM, Timur Tabi wrote:
>> ...
>> ...but then first_chunk() checks the length again, and also returns
>> an Option that is never None.
> 
> In an optimized build, that check is not actually done.  The only checks are for the match
> statement.

Nice. OK.

...
>> Instead, we could use try_from() to check the length and convert
>> the slice, in one step. Your TlvBlockHeader::parse() already uses
>> that approach, and I think it would also work here.
>>
>> Something like this:
>>
>>     pub(crate) fn get_u64(&self, tag: &[u8; 4]) -> Result<u64> {
>>         let value = self.get_bytes(tag)?;
>>
>>         if let Ok(bytes) = <[u8; 4]>::try_from(value) {
>>             Ok(u64::from(u32::from_le_bytes(bytes)))
>>         } else if let Ok(bytes) = <[u8; 8]>::try_from(value) {
>>             Ok(u64::from_le_bytes(bytes))
>>         } else {
>>             Err(EINVAL)
>>         }
>>     }
> 
> I'm more than happy to change it to whatever other people want.  I went through several
> iterations of this code, working with Cursor to evaluate each one.  IMHO, they are all ugly and
> hard to read, because of the from_le_bytes() constraint that I mentioned in the comment.

The reason I responded was that I think the try_from() is nicer
on-screen. I hadn't gone all the way to the asm code.

thanks,
-- 
John Hubbard