Re: [PATCH v3 2/7] gpu: nova-core: add TLV parser for firmware files

On Thu Jul 9, 2026 at 7:48 AM JST, Timur Tabi wrote:
> On Tue, 2026-07-07 at 14:13 +0900, Alexandre Courbot wrote:
>> > Returning error on length==0 simplifies a lot of code on the caller side.
>> 
>> Can you give an example of such a simplification? I'm fine if there is a
>> benefit to doing so, but would like to understand what we gain.
>
> Well, maybe not "a lot" of code, but in general, it eliminates the need to test for empty slices
> every time.  If get_bytes() returns success, you know that you actually do have some bytes.
>
> For example:
>
>         let sig_bytes = self.get_bytes(b"SIGN")?;
>
>         // Ensure that sig_bytes can be divided evenly into chunks.
>         if sig_bytes.len() % num_sigs != 0 {
>             return Err(EINVAL);
>         }
>
>         // num_sigs cannot be 0, and sig_bytes cannot be empty, so this cannot panic.
>         let sig_size = sig_bytes.len() / num_sigs;
>
> sig_bytes.len() cannot be 0, so we know that sig_size cannot be 0, and therefore
>
>         sig_bytes.chunks_exact(sig_size).nth(index).ok_or(EINVAL)
>
> chunks_exact() cannot fail.

Basically we want a guarantee that `sig_size` is not zero. I'd prefer if
we could enforce that using the type system rather than relying on a
guarantee that `get_bytes` doesn't return an empty slice, because even
that guarantee alone is not enough - we also need `num_sigs > 0` and
`sig_bytes.len() % num_sigs == 0` for that.

All these conditions are spread around in the code, so I think it is a
good opportunity to gather them together, and use `NonZero` to hold the
guarantee we want.

Something like this:

    let num_sigs = usize::from_safe_cast(tlv.get_u32(b"NSIG")?);

    // Size of one signature - Booter is always signed
    let sig_size = if (1..=15).contains(&num_sigs) && sig_bytes.len() % num_sigs == 0 {
        NonZero::new(sig_bytes.len() / num_sigs)
    } else {
        None
    }
    .ok_or_else(|| {
        dev_err!(dev, "invalid signature count {}\n", num_sigs);
        EINVAL
    })?;

    ...

    // PANIC: `sig_size` is `NonZero`, so `chunks_exact` cannot panic.
    let sig_chunk = sig_bytes.chunks_exact(sig_size.get()).nth(index).ok_or(EINVAL)?;

This is more sound as the guarantee we need is carried by the type of
`sig_size`, and with that there is no need to make `get_bytes` return an
error on empty slices.