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