| Message ID | f0216ae20b6988514bdb60d8fff96e18c2ce9f1d.1618832277.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | reftable library | expand |
"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.
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.
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 :)
On Wed, Apr 21, 2021 at 11:43 AM Han-Wen Nienhuys <hanwen@google.com> wrote: > > 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 :) This is now the first commit of the reftable series. I think it could graduate separately.
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,