On Mon, Oct 23, 2023 at 06:44:56PM -0400, Taylor Blau wrote:
> +struct bulk_checkin_source {
> + off_t (*read)(struct bulk_checkin_source *, void *, size_t);
> + off_t (*seek)(struct bulk_checkin_source *, off_t);
> +
> + union {
> + struct {
> + int fd;
> + } from_fd;
> + } data;
> +
> + size_t size;
> + const char *path;
> +};
The virtual functions combined with the union are a funny mix of
object-oriented and procedural code. The bulk_checkin_source has
totally virtualized functions, but knows about all of the ancillary data
each set of virtualized functions might want. ;)
I think the more pure OO version would embed the parent, and have each
concrete type define its own struct type, like:
struct bulk_checkin_source_fd {
struct bulk_checkin_source src;
int fd;
};
That works great if the code which constructs it knows which concrete
type it wants, and can just do:
struct bulk_checkin_source_fd src;
init_bulk_checkin_source_from_fd(&src, ...);
If even the construction is somewhat virtualized, then you are stuck
with heap constructors like:
struct bulk_checkin_source *bulk_checkin_source_from_fd(...);
Not too bad, but you have to remember to free now.
Alternatively, I think some of our other OO code just leaves room for
a type-specific void pointer, like:
struct bulk_checkin_source {
...the usual stuff...
void *magic_type_data;
};
and then the init_bulk_checkin_source_from_fd() function allocates its
own heap struct for the magic_type_data field and sticks the int in
there.
That said, both of those are a lot more annoying to use in C (more
boilerplate, more casting, and more opportunities to get something
wrong, including leaks). So I don't mind this in-between state. It is a
funny layering violating from an OO standpoint, but it's not like we
expect an unbounded set of concrete types to "inherit" from the source
struct.
-Peff
