Re: [PATCH v3 4/5] builtin/hash-object: fix uninitialized hash function

[Patrick Steinhardt <ps@pks.im>]

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

On Mon, May 13, 2024 at 04:13:19PM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > From: Patrick Steinhardt <ps@pks.im>
> >
> > The git-hash-object(1) command allows users to hash an object even
> > without a repository. Starting with c8aed5e8da (repository: stop setting
> > SHA1 as the default object hash, 2024-05-07), this will make us hit an
> > uninitialized hash function, which subsequently leads to a segfault.
> >
> > Fix this by falling back to SHA-1 explicitly when running outside of a
> > Git repository. Users can use GIT_DEFAULT_HASH_ALGORITHM environment to
> > specify what hash algorithm they want, so arguably this code should not
> > be needed.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > Signed-off-by: Junio C Hamano <gitster@pobox.com>
> > ---
> >  builtin/hash-object.c   | 3 +++
> >  t/t1007-hash-object.sh  | 6 ++++++
> >  t/t1517-outside-repo.sh | 2 +-
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/builtin/hash-object.c b/builtin/hash-object.c
> > index 82ca6d2bfd..c767414a0c 100644
> > --- a/builtin/hash-object.c
> > +++ b/builtin/hash-object.c
> > @@ -123,6 +123,9 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
> >  	else
> >  		prefix = setup_git_directory_gently(&nongit);
> >  
> > +	if (nongit && !the_hash_algo)
> > +		repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> 
> This is slightly different from your version, which always used SHA-1
> when nongit is true, in the hope that it would make it more useful if
> the user can say "GIT_DEFAULT_HASH_ALGORITHM=sha256 git hash-objects"
> outside a repository.  I am not 100% convinced it is better or we
> rather should aim for predictability that you'd always and only get
> SHA-1 outside a repository.

I'd prefer the latter -- always use SHA-1. As you say, it's easier to
understand and doesn't create implicit mechanisms that we'll have to
maintain going forward. Also, users didn't have a desire yet to pick a
different algorithm than SHA-1, which probably also comes from the fact
that SHA-256 repositories are still scarce.

Eventually, we should then add a new option `--object-hash=` to
git-hash-object(1) and other commands that may run outside of a Git
repository to let the user pick their desired hash.

Patrick

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