Subject: Re: [PATCH v2 2/2] [GSOC] ref-filter: introduce enum atom_type

From: Junio C Hamano <[email-masked]>
Date: Tue, 11 May 2021 22:05:25 +0900

Christian Couder <[email-masked]> writes:

> If it's -1 instead of 0, then it might be a bit more complex to
> initialize structs that contain such a field, as it cannot be done
> with only xcalloc().

In general, yes, but I would have thought that the codepath that
allocates used_atom[] elements are pretty isolated---it is not like
there are random xcalloc() all over the code.  In fact, there is
only one "used_atom_cnt++" in the whole file, which means there is
only one place that (re)allocates the array.

	...
	at = used_atom_cnt;
	used_atom_cnt++;
	REALLOC_ARRAY(used_atom, used_atom_cnt);
	used_atom[at].name = xmemdupz(atom, ep - atom);
	used_atom[at].type = valid_atom[i].cmp_type;
	used_atom[at].source = valid_atom[i].source;
	...

So, I do not think there is even any need to worry about "initialize
to invalid and fill it in as we discover what it really is"; if
there were such a use pattern, UNKNOWN would be handy, but that is
not what we are dealing with here.  In the above snippet, we already
have found from which valid_atom[] element to instantiate the new
element in used_atom[] array.

>> > + * ATOM_INVALID equals to the size of valid_atom array, which could help us
>> > + * iterate over valid_atom array like this:
>> > + *
>> > + *   for (i = ATOM_UNKNOWN + 1; i < ATOM_INVALID; i++) {
>>
>> I find it far more intuitive to say
>>
>>         for (i = 0; i < ATOM_INVALID; i++)
>>
>> than having to say UNKNOWN+1.

And here I was being embarrassingly silly.

As long as we do not waste any entry in valid_atom[] with leading
gap, trailing gap or gap in the middle, the way to iterate over such
an array is

	for (i = 0; i < ARRAY_SIZE(valid_atom); i++)

hence, there is no need for ATOM_MAX, and no need to burden us to
remember that UNKNOWN is near the bottom of the range, and INVALID
is near the top of the range.