On Sat, Mar 24, 2018 at 07:33:47AM +0100, Nguyễn Thái Ngọc Duy wrote: > These delta pointers always point to elements in the objects[] array > in packing_data struct. We can only hold maximum 4G of those objects > because the array size in nr_objects is uint32_t. We could use > uint32_t indexes to address these elements instead of pointers. On > 64-bit architecture (8 bytes per pointer) this would save 4 bytes per > pointer. > > Convert these delta pointers to indexes. Since we need to handle NULL > pointers as well, the index is shifted by one [1]. > > [1] This means we can only index 2^32-2 objects even though nr_objects > could contain 2^32-1 objects. It should not be a problem in > practice because when we grow objects[], nr_alloc would probably > blow up long before nr_objects hits the wall. Hmm, that may be something we eventually fix. I suspect all of this code does some pretty horrible things as you approach 2^32 objects, though. I've never tried to make such a pack, but it may be within the realm of possibility. The .idx file would be 80+GB, but the packfile might not be much bigger if specially crafted. I guess that's outside the realm of reasonable, though, so we can assume that nobody would _really_ want to do that anytime soon. And anything malicious would probably die long before this code triggers. > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> > --- > builtin/pack-objects.c | 116 ++++++++++++++++++++++------------------- > pack-objects.h | 67 ++++++++++++++++++++++-- > 2 files changed, 124 insertions(+), 59 deletions(-) The patch itself looks OK. This is one of the nicer ones, because it really doesn't involve any extra storage management, just some accessor functions. -Peff
