Re: [PATCH v2 2/4] Add structure representing hash algorithm

[Brandon Williams <bmwill@google.com>]

On 10/28, brian m. carlson wrote:
> Since in the future we want to support an additional hash algorithm, add
> a structure that represents a hash algorithm and all the data that must
> go along with it.  Add a constant to allow easy enumeration of hash
> algorithms.  Implement function typedefs to create an abstract API that
> can be used by any hash algorithm, and wrappers for the existing SHA1
> functions that conform to this API.
> 
> Expose a value for hex size as well as binary size.  While one will
> always be twice the other, the two values are both used extremely
> commonly throughout the codebase and providing both leads to improved
> readability.
> 
> Don't include an entry in the hash algorithm structure for the null
> object ID.  As this value is all zeros, any suitably sized all-zero
> object ID can be used, and there's no need to store a given one on a
> per-hash basis.
> 
> The current hash function transition plan envisions a time when we will
> accept input from the user that might be in SHA-1 or in the NewHash
> format.  Since we cannot know which the user has provided, add a
> constant representing the unknown algorithm to allow us to indicate that
> we must look the correct value up.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> I believe I did get the format_id constant for SHA-1 right, but
> sanity-checking would be appreciated.  We don't want another Referer
> mess.
> 
>  cache.h     | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  sha1_file.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+)
> 
> diff --git a/cache.h b/cache.h
> index 6440e2bf21..9e9eb08f05 100644
> --- a/cache.h
> +++ b/cache.h

Maybe it would be good to place this interface in its own header file
that way we can avoid cluttering cache.h with more stuff?

> @@ -77,6 +77,61 @@ struct object_id {
>  	unsigned char hash[GIT_MAX_RAWSZ];
>  };
>  
> +/*
> + * Note that these constants are suitable for indexing the hash_algos array and
> + * comparing against each other, but are otherwise arbitrary, so they should not
> + * be exposed to the user or serialized to disk.  To know whether a
> + * git_hash_algo struct points to some usable hash function, test the format_id
> + * field for being non-zero.  Use the name field for user-visible situations and
> + * the format_id field for fixed-length fields on disk.
> + */
> +/* An unknown hash function. */
> +#define GIT_HASH_UNKNOWN 0
> +/* SHA-1 */
> +#define GIT_HASH_SHA1 1
> +/* Number of algorithms supported (including unknown). */
> +#define GIT_HASH_NALGOS (GIT_HASH_SHA1 + 1)
> +
> +typedef void (*git_hash_init_fn)(void *ctx);
> +typedef void (*git_hash_update_fn)(void *ctx, const void *in, size_t len);
> +typedef void (*git_hash_final_fn)(unsigned char *hash, void *ctx);
> +
> +struct git_hash_algo {
> +	/*
> +	 * The name of the algorithm, as appears in the config file and in
> +	 * messages.
> +	 */
> +	const char *name;
> +
> +	/* A four-byte version identifier, used in pack indices. */
> +	uint32_t format_id;
> +
> +	/* The size of a hash context (e.g. git_SHA_CTX). */
> +	size_t ctxsz;
> +
> +	/* The length of the hash in binary. */
> +	size_t rawsz;
> +
> +	/* The length of the hash in hex characters. */
> +	size_t hexsz;
> +
> +	/* The hash initialization function. */
> +	git_hash_init_fn init_fn;
> +
> +	/* The hash update function. */
> +	git_hash_update_fn update_fn;
> +
> +	/* The hash finalization function. */
> +	git_hash_final_fn final_fn;
> +
> +	/* The OID of the empty tree. */
> +	const struct object_id *empty_tree;
> +
> +	/* The OID of the empty blob. */
> +	const struct object_id *empty_blob;
> +};
> +extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];
> +
>  #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)
>  #define DTYPE(de)	((de)->d_type)
>  #else
> diff --git a/sha1_file.c b/sha1_file.c
> index 10c3a0083d..77b320126a 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -39,6 +39,49 @@ const struct object_id empty_blob_oid = {
>  	EMPTY_BLOB_SHA1_BIN_LITERAL
>  };
>  
> +static inline void git_hash_sha1_init(void *ctx)
> +{
> +	git_SHA1_Init((git_SHA_CTX *)ctx);
> +}
> +
> +static inline void git_hash_sha1_update(void *ctx, const void *data, size_t len)
> +{
> +	git_SHA1_Update((git_SHA_CTX *)ctx, data, len);
> +}
> +
> +static inline void git_hash_sha1_final(unsigned char *hash, void *ctx)
> +{
> +	git_SHA1_Final(hash, (git_SHA_CTX *)ctx);
> +}
> +
> +const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
> +	{
> +		NULL,
> +		0x00000000,
> +		0,
> +		0,
> +		0,
> +		NULL,
> +		NULL,
> +		NULL,
> +		NULL,
> +		NULL,
> +	},
> +	{
> +		"sha-1",
> +		/* "sha1", big-endian */
> +		0x73686131,
> +		sizeof(git_SHA_CTX),
> +		GIT_SHA1_RAWSZ,
> +		GIT_SHA1_HEXSZ,
> +		git_hash_sha1_init,
> +		git_hash_sha1_update,
> +		git_hash_sha1_final,
> +		&empty_tree_oid,
> +		&empty_blob_oid,
> +	},
> +};
> +
>  /*
>   * This is meant to hold a *small* number of objects that you would
>   * want read_sha1_file() to be able to return, but yet you do not want
> -- 
> 2.15.0.rc2
> 

-- 
Brandon Williams