Re: [PATCH 6/9] oid-array: provide a for-loop iterator

From: Jeff King <peff@peff.net>
Date: Dec 4, 2020, 19:51 UTC
Message-ID: <X8qTM22mVzrhqRnn@coredump.intra.peff.net>
In-Reply-To: <X8qIaOe3aT1qr+es@nand.local>

On Fri, Dec 04, 2020 at 02:05:12PM -0500, Taylor Blau wrote:

Show 10 quoted lines

> > +static inline size_t oid_array_next_unique(struct oid_array *array, size_t cur)
> > +{
> > +	do {
> > +		cur++;
> > +	} while (cur < array->nr &&
> > +		 oideq(array->oid + cur, array->oid + cur - 1));
> 
> I don't love the pointer math here (would instead prefer
> oideq(&array->oid[cur]) and so on), but I don't think that it matters
> enough to make a difference.

I think it is a matter of preference. I actually prefer the arithmetic (it's also what was in the code that we are replacing in oid_array_for_each_unique(), but that is probably because I wrote that one, too).

> I additionally had to make sure that cur - 1 >= 0 so that the second
> argument would always be valid, but it is, since we call cur++.

Yeah. I originally wrote it as:

  cur++;
  while (cur < array->nr) {
	if (!oideq(...))
		break;
	cur++;
  }

which maybe makes it more obvious that cur always gets implemented at least once, but I found it harder to reason about the second increment. I think the do-while expresses it pretty clearly, but we're not that used to seeing them.

> You could check that cur++ doesn't overflow, but I think that that's
> mostly academic.

It can't overflow, because it can't ever go past array->nr (unless you call it again after cur is already equal to array->nr, but that would be a bug just like calling i++ after hitting the loop end would be).

-Peff