On Wed, Aug 16, 2017 at 02:24:27PM +0200, Patryk Obara wrote: > On Tue, Aug 15, 2017 at 7:02 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: > >> const int entry_size = GIT_SHA1_HEXSZ + 1; > > > > outside the scope of this patch: > > Is GIT_SHA1_HEXSZ or GIT_MAX_HEXSZ the right call here? > > I think neither one. In my opinion, this code should not be so closely > coupled to hash parsing code - it should be tasked with parsing > whitespace separated list of commit ids without relying on specific > commit id length or format. What I had intended, although maybe I have not explained this well, was that we would have one binary that set up hash functionality as part of early setup. GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ would turn into something like current_hash->rawsz and current_hash->hexsz at that point. The reason I introduced the GIT_MAX constants was to allocate memory suitable for whatever hash we picked. However, this is only what I had considered for design, and others might have different views going forward. I have, however, based my patches on that assumption, and responded to others' comments with those statements. I agree that ideally we should make as much of the code as possible ignorant of the hash size, because that will generally result in more robust, less brittle code. I've noticed in this series the use of parse_oid_hex, and I agree that's one tool we can use to accomplish that goal. -- 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
