Jeff King <peff@xxxxxxxx> writes: > .... But maybe > returning UNKNOWN (as above) is a safer bet than losing the inverse > nature of the by_ptr() and by_id() functions, which could otherwise > cause hard-to-find interactions? Yeah, that sounds sensible. > I think it "works" because the backtrace is: > > [...] > #5 0x0000556d436f6b71 in BUG_fl (file=0x556d43790e8b "hash.h", line=324, > fmt=0x556d43790e73 "null hash, return is %d") at usage.c:335 > #6 0x0000556d4357c2f9 in hash_algo_by_ptr (p=0x0) at /home/peff/compile/git/hash.h:324 > #7 0x0000556d4357c437 in oidclr (oid=0x7ffdf5810ea4, algop=0x0) at /home/peff/compile/git/hash.h:392 > #8 0x0000556d435801c7 in prep_exclude (dir=0x7ffdf5811190, istate=0x556d608959c0, base=0x556d6089da50 "nums", > baselen=0) at dir.c:1699 > [...] > #16 0x0000556d4344dd4a in cmd_grep (argc=0, argv=0x556d60895ee8, prefix=0x0, repo=0x0) at builtin/grep.c:1257 > > So we probably write a totally garbage algo field into the > object_id, but nobody ever ends up looking at it. Probably > something we should clean up, but way out of scope for your series. Yeah, that looks bad, but I think it is fine to leave it outside the series to fix that. > But I do think it reinforces that returning UNKNOWN is an > improvement; it avoids undefined behavior and anybody who tried to > use it should get a BUG() from calling git_hash_unknown_*() > functions. Sounds like a sensitive direction to go in. Thanks.
