Re: [PATCH v6 4/5] Add reftable library
- From
Jeff King <peff@peff.net>
- Date
- Feb 19, 2020, 06:55 UTC
- Message-ID
- <20200219065533.GA623000@coredump.intra.peff.net>
- In-Reply-To
- <xmqqtv3niosf.fsf@gitster-ct.c.googlers.com>
On Tue, Feb 18, 2020 at 01:11:28PM -0800, Junio C Hamano wrote:
Show 12 quoted lines
> "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +void put_u64(byte *out, uint64_t v)
> > +{
> > + int i = 0;
> > + for (i = sizeof(uint64_t); i--;) {
> > + out[i] = (byte)(v & 0xff);
> > + v >>= 8;
> > + }
> > +}
>
> This looks OK, but ...There's a useless initialization of "i" there, I think?
It also seems weird to decrement "i" in the loop condition and leave the final field of the for-loop empty. Would:
int i = sizeof(uint64_t);
while (i--) {
...
}be more clear? But...
We also have put_be64() already (it's coded without a loop, though I'd imagine just about any compiler would unroll the loop above).
I wonder how possible it would be to use the Git names for these utility functions in the reftable library, and then push the common ones out into a compat file within the reftable library. Then we could toss out the compat file when importing the library to Git, but it could still compile standalone.
-Peff