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

From: Junio C Hamano <gitster@pobox.com>
Date: May 11, 2021, 13:05 UTC
Message-ID: <xmqqbl9hmne2.fsf@gitster.g>
In-Reply-To: <CAP8UFD3m0TA3nPDaPhaU3=FVF_r5cPM8xksVL6MpCOhNC4dT3g@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> 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.

Show 10 quoted lines

>> > + * 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.