# [PATCH] packfile: use get_be64() for large offsets 5 messages from 2018-01-17 to 2018-01-19. Participants: Derrick Stolee, Jeff King, SZEDER Gábor, Junio C Hamano. Thread: https://gitlist.dev/t/47627 ## Derrick Stolee, 2018-01-17 19:08 Subject: [PATCH] packfile: use get_be64() for large offsets Message-ID: <20180117190823.209469-1-dstolee@microsoft.com> URL: https://gitlist.dev/e/20180117190823.209469-1-dstolee%40microsoft.com ``` The pack-index version 2 format uses two 4-byte integers in network-byte order to represent one 8-byte value. The current implementation has several code clones for stitching these integers together. Use get_be64() to create an 8-byte integer from two 4-byte integers represented this way. Signed-off-by: Derrick Stolee --- pack-revindex.c | 6 ++---- packfile.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/pack-revindex.c b/pack-revindex.c index 1b7ebd8d7e..ff5f62c033 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -134,10 +134,8 @@ static void create_pack_revindex(struct packed_git *p) if (!(off & 0x80000000)) { p->revindex[i].offset = off; } else { - p->revindex[i].offset = - ((uint64_t)ntohl(*off_64++)) << 32; - p->revindex[i].offset |= - ntohl(*off_64++); + p->revindex[i].offset = get_be64(off_64); + off_64 += 2; } p->revindex[i].nr = i; } diff --git a/packfile.c b/packfile.c index 4a5fe7ab18..228ed0d59a 100644 --- a/packfile.c +++ b/packfile.c @@ -1702,8 +1702,7 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n) return off; index += p->num_objects * 4 + (off & 0x7fffffff) * 8; check_pack_index_ptr(p, index); - return (((uint64_t)ntohl(*((uint32_t *)(index + 0)))) << 32) | - ntohl(*((uint32_t *)(index + 4))); + return get_be64(index); } } -- 2.15.0 ``` ## Jeff King, 2018-01-17 22:29 Subject: Re: [PATCH] packfile: use get_be64() for large offsets Message-ID: <20180117222953.GB16031@sigill.intra.peff.net> URL: https://gitlist.dev/e/20180117222953.GB16031%40sigill.intra.peff.net In-Reply-To: <20180117190823.209469-1-dstolee@microsoft.com> ``` On Wed, Jan 17, 2018 at 02:08:23PM -0500, Derrick Stolee wrote: > The pack-index version 2 format uses two 4-byte integers in network-byte order to represent one 8-byte value. The current implementation has several code clones for stitching these integers together. Yeah, this seems like a no-brainer, and the patch itself looks obviously correct. Please wrap your commit messages a bit shorter, though (say, between 60-72 chars). -Peff ``` ## Derrick Stolee, 2018-01-18 11:04 Subject: [PATCH v2] packfile: use get_be64() for large offsets Message-ID: <20180118110419.210006-1-dstolee@microsoft.com> URL: https://gitlist.dev/e/20180118110419.210006-1-dstolee%40microsoft.com In-Reply-To: <20180117222953.GB16031@sigill.intra.peff.net> ``` The pack-index version 2 format uses two 4-byte integers in network- byte order to represent one 8-byte value. The current implementation has several code clones for stitching these integers together. Use get_be64() to create an 8-byte integer from two 4-byte integers represented this way. Signed-off-by: Derrick Stolee --- pack-revindex.c | 6 ++---- packfile.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/pack-revindex.c b/pack-revindex.c index 1b7ebd8d7e..ff5f62c033 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -134,10 +134,8 @@ static void create_pack_revindex(struct packed_git *p) if (!(off & 0x80000000)) { p->revindex[i].offset = off; } else { - p->revindex[i].offset = - ((uint64_t)ntohl(*off_64++)) << 32; - p->revindex[i].offset |= - ntohl(*off_64++); + p->revindex[i].offset = get_be64(off_64); + off_64 += 2; } p->revindex[i].nr = i; } diff --git a/packfile.c b/packfile.c index 4a5fe7ab18..228ed0d59a 100644 --- a/packfile.c +++ b/packfile.c @@ -1702,8 +1702,7 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n) return off; index += p->num_objects * 4 + (off & 0x7fffffff) * 8; check_pack_index_ptr(p, index); - return (((uint64_t)ntohl(*((uint32_t *)(index + 0)))) << 32) | - ntohl(*((uint32_t *)(index + 4))); + return get_be64(index); } } -- 2.15.0 ``` ## SZEDER Gábor, 2018-01-18 14:15 Subject: Re: [PATCH v2] packfile: use get_be64() for large offsets Message-ID: <20180118141522.18294-1-szeder.dev@gmail.com> URL: https://gitlist.dev/e/20180118141522.18294-1-szeder.dev%40gmail.com In-Reply-To: <20180118110419.210006-1-dstolee@microsoft.com> ``` Junio, > The pack-index version 2 format uses two 4-byte integers in network- > byte order to represent one 8-byte value. The current implementation > has several code clones for stitching these integers together. > > Use get_be64() to create an 8-byte integer from two 4-byte integers > represented this way. > > Signed-off-by: Derrick Stolee > --- > pack-revindex.c | 6 ++---- > packfile.c | 3 +-- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/pack-revindex.c b/pack-revindex.c > index 1b7ebd8d7e..ff5f62c033 100644 > --- a/pack-revindex.c > +++ b/pack-revindex.c > @@ -134,10 +134,8 @@ static void create_pack_revindex(struct packed_git *p) > if (!(off & 0x80000000)) { > p->revindex[i].offset = off; > } else { > - p->revindex[i].offset = > - ((uint64_t)ntohl(*off_64++)) << 32; > - p->revindex[i].offset |= > - ntohl(*off_64++); > + p->revindex[i].offset = get_be64(off_64); > + off_64 += 2; > } > p->revindex[i].nr = i; > } > diff --git a/packfile.c b/packfile.c > index 4a5fe7ab18..228ed0d59a 100644 > --- a/packfile.c > +++ b/packfile.c > @@ -1702,8 +1702,7 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n) > return off; > index += p->num_objects * 4 + (off & 0x7fffffff) * 8; > check_pack_index_ptr(p, index); > - return (((uint64_t)ntohl(*((uint32_t *)(index + 0)))) << 32) | > - ntohl(*((uint32_t *)(index + 4))); > + return get_be64(index); > } > } > > -- > 2.15.0 > This patch can't be applied to 'maint' currently at 3013dff86 (Prepare for 2.15.2, 2017-12-06), as it is in case of 'ds/use-get-be64', because 'maint' doesn't have get_be64() yet (b2e39d006 (bswap: add 64 bit endianness helper get_be64, 2017-09-22)). Gábor ``` ## Junio C Hamano, 2018-01-19 19:12 Subject: Re: [PATCH v2] packfile: use get_be64() for large offsets Message-ID: URL: https://gitlist.dev/e/xmqqvafxpsru.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20180118141522.18294-1-szeder.dev@gmail.com> ``` SZEDER Gábor writes: > Junio, > ... > This patch can't be applied to 'maint' currently at 3013dff86 (Prepare > for 2.15.2, 2017-12-06), as it is in case of 'ds/use-get-be64', > because 'maint' doesn't have get_be64() yet (b2e39d006 (bswap: add 64 > bit endianness helper get_be64, 2017-09-22)). Thanks for stopping me. I do not always get to test anything not in 'next' in isolation before I actually try to merge it there (at which time I would have noticed), and you caught me in this one doing a short-cut "just queue it somewhere and throw it in 'pu'". ```