diff mbox series

[v7,04/28] hash.h: provide constants for the hash IDs

Message ID f0216ae20b6988514bdb60d8fff96e18c2ce9f1d.1618832277.git.gitgitgadget@gmail.com (mailing list archive)
State New
Headers show
Series reftable library | expand

Commit Message

Han-Wen Nienhuys April 19, 2021, 11:37 a.m. UTC
From: Han-Wen Nienhuys <hanwen@google.com>

This will simplify referencing them from code that is not deeply integrated with
Git, in particular, the reftable library.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 hash.h        | 6 ++++++
 object-file.c | 6 ++----
 2 files changed, 8 insertions(+), 4 deletions(-)

Comments

Junio C Hamano April 20, 2021, 7:49 p.m. UTC | #1
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> This will simplify referencing them from code that is not deeply integrated with
> Git, in particular, the reftable library.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  hash.h        | 6 ++++++
>  object-file.c | 6 ++----
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hash.h b/hash.h
> index 3fb0c3d4005a..b17fb2927711 100644
> --- a/hash.h
> +++ b/hash.h
> @@ -161,12 +161,18 @@ static inline int hash_algo_by_ptr(const struct git_hash_algo *p)
>  	return p - hash_algos;
>  }
>  
> +/* "sha1", big-endian */
> +#define GIT_SHA1_HASH_ID 0x73686131
> +
>  /* The length in bytes and in hex digits of an object name (SHA-1 value). */
>  #define GIT_SHA1_RAWSZ 20
>  #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
>  /* The block size of SHA-1. */
>  #define GIT_SHA1_BLKSZ 64
>  
> +/* "s256", big-endian */
> +#define GIT_SHA256_HASH_ID 0x73323536

I agree that it makes sense to have symbolic constants defined in
this file.  These are values that fit in the ".format_id" member of
"struct git_hash_algo", and it is preferrable to make sure that the
names align (if I were designing in void, I would have called the
member "algo_id" and made the constants GIT_(SHA1|SHA256)_ALGO_ID).

Brian?  What's your preference ("I am fine to store HASH_ID in the
'.format_id' member" is perfectly an acceptable answer).

Thanks.
brian m. carlson April 21, 2021, 1:04 a.m. UTC | #2
On 2021-04-20 at 19:49:41, Junio C Hamano wrote:
> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > From: Han-Wen Nienhuys <hanwen@google.com>
> >
> > This will simplify referencing them from code that is not deeply integrated with
> > Git, in particular, the reftable library.
> >
> > Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> > ---
> >  hash.h        | 6 ++++++
> >  object-file.c | 6 ++----
> >  2 files changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/hash.h b/hash.h
> > index 3fb0c3d4005a..b17fb2927711 100644
> > --- a/hash.h
> > +++ b/hash.h
> > @@ -161,12 +161,18 @@ static inline int hash_algo_by_ptr(const struct git_hash_algo *p)
> >  	return p - hash_algos;
> >  }
> >  
> > +/* "sha1", big-endian */
> > +#define GIT_SHA1_HASH_ID 0x73686131
> > +
> >  /* The length in bytes and in hex digits of an object name (SHA-1 value). */
> >  #define GIT_SHA1_RAWSZ 20
> >  #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
> >  /* The block size of SHA-1. */
> >  #define GIT_SHA1_BLKSZ 64
> >  
> > +/* "s256", big-endian */
> > +#define GIT_SHA256_HASH_ID 0x73323536
> 
> I agree that it makes sense to have symbolic constants defined in
> this file.  These are values that fit in the ".format_id" member of
> "struct git_hash_algo", and it is preferrable to make sure that the
> names align (if I were designing in void, I would have called the
> member "algo_id" and made the constants GIT_(SHA1|SHA256)_ALGO_ID).
> 
> Brian?  What's your preference ("I am fine to store HASH_ID in the
> '.format_id' member" is perfectly an acceptable answer).

I slightly prefer FORMAT_ID because it's consistent (and for that reason
only), but if HASH_ID is more convenient, that's fine; I don't have a
strong opinion at all.  Definitely don't reroll the series because of my
slight preference.  Either way, I think these are fine things to have as
constants, and I appreciate you hoisting the comments here.
Han-Wen Nienhuys April 21, 2021, 9:43 a.m. UTC | #3
On Wed, Apr 21, 2021 at 3:04 AM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> > I agree that it makes sense to have symbolic constants defined in
> > this file.  These are values that fit in the ".format_id" member of
> > "struct git_hash_algo", and it is preferrable to make sure that the
> > names align (if I were designing in void, I would have called the
> > member "algo_id" and made the constants GIT_(SHA1|SHA256)_ALGO_ID).
> >
> > Brian?  What's your preference ("I am fine to store HASH_ID in the
> > '.format_id' member" is perfectly an acceptable answer).
>
> I slightly prefer FORMAT_ID because it's consistent (and for that reason
> only), but if HASH_ID is more convenient, that's fine; I don't have a
> strong opinion at all.  Definitely don't reroll the series because of my
> slight preference.  Either way, I think these are fine things to have as
> constants, and I appreciate you hoisting the comments here.

Either way is fine for me. I can paint the bikeshed in your favorite color :)
diff mbox series

Patch

diff --git a/hash.h b/hash.h
index 3fb0c3d4005a..b17fb2927711 100644
--- a/hash.h
+++ b/hash.h
@@ -161,12 +161,18 @@  static inline int hash_algo_by_ptr(const struct git_hash_algo *p)
 	return p - hash_algos;
 }
 
+/* "sha1", big-endian */
+#define GIT_SHA1_HASH_ID 0x73686131
+
 /* The length in bytes and in hex digits of an object name (SHA-1 value). */
 #define GIT_SHA1_RAWSZ 20
 #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
 /* The block size of SHA-1. */
 #define GIT_SHA1_BLKSZ 64
 
+/* "s256", big-endian */
+#define GIT_SHA256_HASH_ID 0x73323536
+
 /* The length in bytes and in hex digits of an object name (SHA-256 value). */
 #define GIT_SHA256_RAWSZ 32
 #define GIT_SHA256_HEXSZ (2 * GIT_SHA256_RAWSZ)
diff --git a/object-file.c b/object-file.c
index 624af408cdcd..5af384a8cfc4 100644
--- a/object-file.c
+++ b/object-file.c
@@ -146,8 +146,7 @@  const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 	},
 	{
 		"sha1",
-		/* "sha1", big-endian */
-		0x73686131,
+		GIT_SHA1_HASH_ID,
 		GIT_SHA1_RAWSZ,
 		GIT_SHA1_HEXSZ,
 		GIT_SHA1_BLKSZ,
@@ -160,8 +159,7 @@  const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
 	},
 	{
 		"sha256",
-		/* "s256", big-endian */
-		0x73323536,
+		GIT_SHA256_HASH_ID,
 		GIT_SHA256_RAWSZ,
 		GIT_SHA256_HEXSZ,
 		GIT_SHA256_BLKSZ,