Jeff King <peff@xxxxxxxx> writes:
> On Thu, Oct 19, 2023 at 01:28:42PM -0400, Taylor Blau wrote:
>
>> +struct bulk_checkin_source {
>> + enum { SOURCE_FILE } type;
>> +
>> + /* SOURCE_FILE fields */
>> + int fd;
>> +
>> + /* common fields */
>> + size_t size;
>> + const char *path;
>> +};
>
> This is a pretty minor nit, but we may find that "SOURCE_FILE" is not
> sufficiently name-spaced. Enum tags are in the global namespace, so
> the compiler will barf if there are any conflicts.
A very good point. This was one of the reasons why I suggested
avoiding the switch() based on a new enum altogether and use a
vtable based approach instead. But it may do while this one is
private to the file and never exposed to other subsystems.
> It might be OK here, since this is local to a single C file (so we at
> least are not hurting other code), but we may be in trouble if code in a
> header file is less careful. There is already a near-miss here with
> GREP_SOURCE_FILE, but fortunately grep.h is indeed careful. :)
>
> (I notice that ref-filter.c similarly uses SOURCE_* for an internal
> enum).
Thanks.
