On Sat, Oct 28, 2017 at 09:44:07PM -0400, Eric Sunshine wrote:
> > diff --git a/cache.h b/cache.h
> > @@ -132,6 +133,8 @@ struct git_hash_algo {
> > extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];
> >
> > +#define current_hash the_repository->hash_algo
>
> The all-lowercase name "current_hash" seems likely to conflict with a
> variable name some day; the fact that it is also a #define makes such
> a collision even more worrisome. Although it is retrieving the "hash
> algorithm", when reading the terse name "current_hash", one may
> instead intuitively think it is referring to a hash _value_ (not an
> algorithm).
I can do CURRENT_HASH_ALGO or CURRENT_HASH instead if you think that's
an improvement. I originally omitted the "algo" portion to keep it
short.
Alternatively, we could have a current_hash() (or current_hash_algo())
inline function if people like that better.
> > * Attempt to resolve and set the provided 'gitdir' for repository 'repo'.
> > * Return 0 upon success and a non-zero value upon failure.
> > @@ -136,6 +141,8 @@ int repo_init(struct repository *repo, const char *gitdir, const char *worktree)
> > if (read_and_verify_repository_format(&format, repo->commondir))
> > goto error;
> >
> > + repo->hash_algo = &hash_algos[format.hash_algo];
>
> Should this be instead invoking repo_set_hash_algo()?
Yes, it should. Will fix.
--
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
Attachment:
signature.asc
Description: PGP signature
