Re: [PATCH v1] hash: handle invalid algorithms when formatting object IDs

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 1648 bytes --]

On 2026-11-05 at 14:47:00, Tanga Joris wrote:
> Hello shrimech,
> 
> Thank you for the patch.
> 
> I tried it on top of master (0ffa3e35c7). "cargo test" passes, and
> the new test fails without your change to fmt(), so it does catch
> the problem.
> 
> On Tue, 3 Nov 2026 at 07:42, shrimech <eddinen77@gmail.com> wrote:
> > +            Err(e) => write!(f, ":invalid-hash-algo-{}", e.0),
> 
> I have one question. In the C code, algo 0 is not treated as an
> error when printing: hash_to_hex_algop_r() in hex.c falls back to
> the default hash ("Our struct object_id has been memset to 0, so
> default to printing using the default hash"). Your test uses algo 0
> as the invalid case, so for algo 0 the Rust output would be
> different from C. Is that intended?

This is not a valid configuration.  I have a patch on my sha256-interop
branch, 5b208e4a3d34ffba0fbeafe991643d6d47a5caa0, that should make the
members not public.  It depends partially on things other in that
series, but it should be fine to drop those pieces if you want to pick
that up and send it in.

If you pass the structure from C, it's intended that it panics if algo
is zero.  That's because we don't want to rely on the behaviour of
depending on a mutable global in Rust, since that's unsafe and leads to
action at a distance.  We really should just drop that fallback and fix
all of the relevant callers at some point.

> brian m. carlson wrote most of src/hash.rs, so I have added him on
> Cc, I hope that is okay:)

Minor nit: I use they/them pronouns.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA
Speaking for myself only

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]