| Message ID | d7cbd4ca1a5dd63fb1d19ef97fac9765daa5ae03.1599172908.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | more miscellaneous Bloom filter improvements | expand |
Am 04.09.20 um 00:46 schrieb Taylor Blau: > A small handful of writers who wish to encode 64-bit values in network > order have worked around the lack of such a helper by calling the 32-bit > variant twice. > > The subsequent commit will add another caller who wants to write a > 64-bit value. To ease their (and the existing caller's) pain, introduce > a helper to do just that, and convert existing call-sites. > > Suggested-by: Derrick Stolee <dstolee@microsoft.com> > Signed-off-by: Taylor Blau <me@ttaylorr.com> > --- > commit-graph.c | 8 ++------ > csum-file.h | 6 ++++++ > midx.c | 3 +-- > 3 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/commit-graph.c b/commit-graph.c > index 35535f4192..01d791343a 100644 > --- a/commit-graph.c > +++ b/commit-graph.c > @@ -1791,12 +1791,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) > > chunk_offset = 8 + (num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH; > for (i = 0; i <= num_chunks; i++) { > - uint32_t chunk_write[3]; > - > - chunk_write[0] = htonl(chunks[i].id); > - chunk_write[1] = htonl(chunk_offset >> 32); > - chunk_write[2] = htonl(chunk_offset & 0xffffffff); > - hashwrite(f, chunk_write, 12); > + hashwrite_be32(f, chunks[i].id); > + hashwrite_be64(f, chunk_offset); > > chunk_offset += chunks[i].size; > } > diff --git a/csum-file.h b/csum-file.h > index f9cbd317fb..b026ec7766 100644 > --- a/csum-file.h > +++ b/csum-file.h > @@ -62,4 +62,10 @@ static inline void hashwrite_be32(struct hashfile *f, uint32_t data) > hashwrite(f, &data, sizeof(data)); > } > > +static inline void hashwrite_be64(struct hashfile *f, uint64_t data) > +{ > + hashwrite_be32(f, data >> 32); > + hashwrite_be32(f, data & 0xffffffffUL); > +} > + > #endif > diff --git a/midx.c b/midx.c > index e9b2e1253a..32cc5fdc22 100644 > --- a/midx.c > +++ b/midx.c > @@ -789,8 +789,7 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off > if (!(offset >> 31)) > continue; > > - hashwrite_be32(f, offset >> 32); > - hashwrite_be32(f, offset & 0xffffffffUL); > + hashwrite_be64(f, offset); > written += 2 * sizeof(uint32_t); "2 * sizeof(uint32_t)" looks slightly out of sync with the hashwrite_be64() call now; "sizeof(uint64_t)" would be more fitting. > > nr_large_offset--; > There's also this potential caller: midx.c=802=static int write_midx_internal(const char *object_dir, struct multi_pack_index *m, midx.c:981: hashwrite_be32(f, chunk_ids[i]); midx.c:982: hashwrite_be32(f, chunk_offsets[i] >> 32); midx.c:983: hashwrite_be32(f, chunk_offsets[i]); Not sure it's worth a reroll, though. (I'd probably leave those conversions for a later series.) René
On Fri, Sep 04, 2020 at 10:18:38PM +0200, René Scharfe wrote: > Am 04.09.20 um 00:46 schrieb Taylor Blau: > "2 * sizeof(uint32_t)" looks slightly out of sync with the hashwrite_be64() > call now; "sizeof(uint64_t)" would be more fitting. Yeah, agreed. > > > > nr_large_offset--; > > > > There's also this potential caller: > > midx.c=802=static int write_midx_internal(const char *object_dir, struct multi_pack_index *m, > midx.c:981: hashwrite_be32(f, chunk_ids[i]); > midx.c:982: hashwrite_be32(f, chunk_offsets[i] >> 32); > midx.c:983: hashwrite_be32(f, chunk_offsets[i]); > > Not sure it's worth a reroll, though. > > (I'd probably leave those conversions for a later series.) Agreed. If we were earlier on, or there wasn't already a patch that I had swapped out for a manual fixup after sending this v4, I'd certainly fold these in, but I think at this point it's easier to apply this separately on top. Thanks for pointing them out. > René Thanks, Taylor
diff --git a/commit-graph.c b/commit-graph.c index 35535f4192..01d791343a 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1791,12 +1791,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) chunk_offset = 8 + (num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH; for (i = 0; i <= num_chunks; i++) { - uint32_t chunk_write[3]; - - chunk_write[0] = htonl(chunks[i].id); - chunk_write[1] = htonl(chunk_offset >> 32); - chunk_write[2] = htonl(chunk_offset & 0xffffffff); - hashwrite(f, chunk_write, 12); + hashwrite_be32(f, chunks[i].id); + hashwrite_be64(f, chunk_offset); chunk_offset += chunks[i].size; } diff --git a/csum-file.h b/csum-file.h index f9cbd317fb..b026ec7766 100644 --- a/csum-file.h +++ b/csum-file.h @@ -62,4 +62,10 @@ static inline void hashwrite_be32(struct hashfile *f, uint32_t data) hashwrite(f, &data, sizeof(data)); } +static inline void hashwrite_be64(struct hashfile *f, uint64_t data) +{ + hashwrite_be32(f, data >> 32); + hashwrite_be32(f, data & 0xffffffffUL); +} + #endif diff --git a/midx.c b/midx.c index e9b2e1253a..32cc5fdc22 100644 --- a/midx.c +++ b/midx.c @@ -789,8 +789,7 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off if (!(offset >> 31)) continue; - hashwrite_be32(f, offset >> 32); - hashwrite_be32(f, offset & 0xffffffffUL); + hashwrite_be64(f, offset); written += 2 * sizeof(uint32_t); nr_large_offset--;
A small handful of writers who wish to encode 64-bit values in network order have worked around the lack of such a helper by calling the 32-bit variant twice. The subsequent commit will add another caller who wants to write a 64-bit value. To ease their (and the existing caller's) pain, introduce a helper to do just that, and convert existing call-sites. Suggested-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- commit-graph.c | 8 ++------ csum-file.h | 6 ++++++ midx.c | 3 +-- 3 files changed, 9 insertions(+), 8 deletions(-)