Re: [PATCH] Use MOVE_ARRAY

From: Jeff King <peff@peff.net>
Date: Jan 22, 2018, 23:34 UTC
Message-ID: <20180122233426.GA27144@sigill.intra.peff.net>
In-Reply-To: <xmqq372xjwzg.fsf@gitster.mtv.corp.google.com>

On Mon, Jan 22, 2018 at 03:26:59PM -0800, Junio C Hamano wrote:

Show 17 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Most of these are "shift part of the array". I wonder if it would make
> > sense to encapsulate that pattern in a helper, like:
> >
> >   #define SHIFT_ARRAY(a, nr, pos, slots) \
> >     MOVE_ARRAY(a + pos + slots, a + pos, nr - pos - slots)
> >   ...
> >   SHIFT_ARRAY(it->down, it->subtree_nr, pos, 1);
> 
> Exactly my thought when I was queuing it, but I was wondering about
> this more from "can we use the higher level abstraction for reducing
> errors?" point of view.  If we are shifting an array by 3 slots to
> the right, we should at least have enough slots allocated to hold
> them (i.e. a->nr - a->alloc must be 3 or more).  But after realizing
> that the level these macros operate at is still a bit too low to do
> something like that, I quickly lost interest ;-)

Yeah, you'd need to know the "alloc" number to right-shift correctly, since by definition we're pushing off the end of a->nr. Left-shifts just need to make sure they don't go past "0", which we can do here, but I'd think they're pretty uncommon.

The right macro level may actually be something more like "make room for N items at pos". E.g.:

  #define CREATE_ARRAY_HOLE(a, nr, alloc, pos, slots) do { \
          ALLOC_GROW(a, nr + slots, alloc);
	  MOVE_ARRAY(a + pos + slots, a + pos, nr - pos - slots);
  } while (0)

but I didn't investigate the surrounding code. And it surely would need a catchier name. ;)

-Peff