Re: [PATCH 2/5] roaring.[ch]: apply Git specific changes to the roaring API

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 19, 2022, 22:02 UTC
Message-ID: <xmqqr10781lx.fsf@gitster.g>
In-Reply-To: <b727c25c-469f-ca56-bbd6-82f82c762523@github.com>

Derrick Stolee <derrickstolee@github.com> writes:

Show 8 quoted lines

>>  int32_t array_container_write(const array_container_t *container, char *buf);
>> +
>> +int array_container_network_write(const array_container_t *container,
>> +				  int (*write_fn) (void *, const void *, size_t),
>> +				  void *data);
>
> Should we make write_fn a defined type? I'm not sure I've seen this
> implicit type within a function declaration before.

Unless we can point out why having a named type is a good idea (e.g. we add such a function pointer as a member of a struct, or we keep a variable of that type somewhere), I actually would prefer to do without them.

Perhaps there are some more important reasons I am missing why we often come up with explicit types for callback function pointers in many parts of our API, but if there aren't, my preference actually is to lose them, not add more of them.

Hmph.... could "a typedef can become a place to give definitive documentation for the class of callback functions" be a good reason why we would want one? I dunno.

In the posted patch, readers cannot tell what kind of three parameters they are supposed to give to write_fn().

Thanks.