packfile: use get_be64() for large offsets
PATCH 1/1 packfile: use get_be64() for large offsets
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.
--- pack-revindex.c | 6 ++---- packfile.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)
pack-revindex.c +2 -4
@@ -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; }
packfile.c +1 -2
@@ -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); } }
Signature · 1 line
2.15.0
Quoted 1 line · Derrick Stolee
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
PATCH v2 1/1 packfile: use get_be64() for large offsets
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.
--- pack-revindex.c | 6 ++---- packfile.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)
pack-revindex.c +2 -4
@@ -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; }
packfile.c +1 -2
@@ -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); } }
Signature · 1 line
2.15.0
Junio,
Quoted 47 lines
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 <[email protected]>--- pack-revindex.c | 6 ++---- packfile.c | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-)pack-revindex.c +2
-4@@ -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; }packfile.c +1
-2@@ -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); } }Signature · 1 line
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
Quoted 6 lines · SZEDER Gábor
Junio,
...
This patch can't be applied to 'maint' currently at3013dff86(Prepare for 2.15.2, 2017-12-06), as it is in case of 'ds/use-get-be64', because 'maint' doesn't haveget_be64()yet (b2e39d006(bswap: add 64 bit endianness helperget_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'".