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

From: Junio C Hamano <[email-masked]>
Date: Mon, 19 Sep 2022 15:02:02 -0700

Derrick Stolee <[email-masked]> writes:

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