Subject: Re: [PATCH 05/14] rust: add a hash algorithm abstraction

From: Junio C Hamano <[email-masked]>
Date: Wed, 29 Oct 2025 06:27:17 -0700

Ezekiel Newren <[email-masked]> writes:

>> > +impl ObjectID {
>> > +    pub fn as_slice(&self) -> &[u8] {
>> > +        match HashAlgorithm::from_u32(self.algo) {
>> > +            Some(algo) => &self.hash[0..algo.raw_len()],
>> > +            None => &self.hash,
>> > +        }
>> > +    }
>> > +
>> > +    pub fn as_mut_slice(&mut self) -> &mut [u8] {
>> > +        match HashAlgorithm::from_u32(self.algo) {
>> > +            Some(algo) => &mut self.hash[0..algo.raw_len()],
>> > +            None => &mut self.hash,
>> > +        }
>> > +    }
>> > +}
>>
>> These cases for "None" surprised me a bit; I would have expected us
>> to error out when given an algorithm we do not recognise.
>
> I think _Result_ would be more appropriate here.

Perhaps.  But the Option/Result was not what I was suprised about.

When algo is available, we gave back a slice that is properly sized,
but when algo is not, I would have expected it to say "nope",
instead of yielding the full area of memory available.  That was the
part I was surprised about.

Perhaps as_mut_slice() side is justifiable (an uninitialized
instance of ObjectID is filled by getting the full self.hash and
filling it, plus filling the algo), but the same explanation would
not apply on the read-only side.