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

Patrick Steinhardt [off-list ref] writes:

quoted

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.