Re: [PATCH v2 3/4] Integrate hash algorithm support with repo setup

From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Oct 29, 2017, 17:57 UTC
Message-ID: <20171029175712.ijqirnghcgeufqae@genre.crustytoothpaste.net>
In-Reply-To: <CAPig+cTrAxWczJ5qX2qE-hqoo6hA2QCom25HYB7v48rVnPgTow@mail.gmail.com>

On Sat, Oct 28, 2017 at 09:44:07PM -0400, Eric Sunshine wrote:

Show 12 quoted lines

> > 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.

Show 9 quoted lines

> >   * 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