Re: [PATCH v3 3/5] builtin/patch-id: fix uninitialized hash function

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>> Hmph, in other places I did
>> 
>> 	if (!the_hash_algo)
>> 		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
>> 
>> to find the case where we need a reasonable default.
>> 
>> Is there a practical difference?  If there isn't we should
>> standardise one and use the same test consistently everywhere.
>> ...
>
> To the best of my knowledge there isn't. What I prefer about my approach
> is that it explicitly points out that this is conditional on whether or
> not we have a repository. But in the end I don't mind much which of both
> versions we use.

Ah, that makes sense, and it is quite subjective but makes certain
sense.

The reason I prefered to check "the_hash_algo" is very much the
opposite.  In this particular decision to call (or not call)
set_hash_algo(), we only care if the_hash_algo is not yet set, and
that is why the_hash_algo is checked.

Specifically, we do not care *why* it is still unset; in the current
codebase, the most likely reason why we do not have the_hash_algo
set might be that we haven't found the repository yet, but we do not
have to rely on that assumption to hold true.  It would help
maintainability into the future where the_hash_algo is set already
before we come here when outside a repository, or vice versa.