Re: [PATCH v7 07/13] pack-objects: refer to delta objects by index instead of pointer

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 <redacted>
---
 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