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

From: Taylor Blau <me@ttaylorr.com>
Date: Dec 4, 2020, 19:05 UTC
Message-ID: <X8qIaOe3aT1qr+es@nand.local>
In-Reply-To: <X8qFo+GJJTbaPV58@coredump.intra.peff.net>

On Fri, Dec 04, 2020 at 01:53:23PM -0500, Jeff King wrote:

> I also considered adding a full iterator type with init/next/end
> functions (similar to what we have for hashmaps). But it ended up making
> the callers much harder to read. This version keeps us close to a basic
> for-loop.

Yeah, I think that a full-blown iterator type is overkill for this purpose. Another possible approach could be a macro:

  #define for_each_oid_array_unique(arr, i) \
    for (i = 0; (i) < (array)->nr; i = oid_array_for_each_unique((arr), i))

but I don't think that's making anything more clear than 'oid_array_for_each_unique' already is. So, I like the approach that you took here.

Show 21 quoted lines

> @@ -111,4 +113,24 @@ void oid_array_filter(struct oid_array *array,
>   */
>  void oid_array_sort(struct oid_array *array);
>
> +/**
> + * Find the next unique oid in the array after position "cur". You
> + * can use this to iterate over unique elements, like:
> + *
> + *   size_t i;
> + *   oid_array_sort(array);
> + *   for (i = 0; i < array->nr; i = oid_array_next_unique(array, i))
> + *	printf("%s", oid_to_hex(array->oids[i]);
> + *
> + * Non-unique iteration can just increment with "i++" to visit each element.
> + */
> +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 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++.

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

Thanks, Taylor