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.
> @@ -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
