On 2/12/20 8:26 AM, Richard W.M. Jones wrote:
> With this change, 'next_open' and '(int (*)(void *,
> int))backend_open' now have identical semantics. I'm trying to see
> if there are further changes we could make that would alleviate the
> need for function casts. I don't know if it is worth changing
> nbdkit-filter.h to use 'struct backend *' instead of 'void *' (while
> leaving struct backend an incomplete type in the public header) - it
> would be ABI compatible, and although it would require
> recompilation, we already state that filter recompilation is par for
> the course (since only plugins promise API compatibility).
Yes my original version had stuff like:
static struct nbdkit_next_ops next_ops = {
.reopen = (void *) backend_reopen,
.get_size = (void *) backend_get_size,
but that wasn't very safe,
Yeah, the cast through (void*) is very imprecise, and a more precise
cast like (int (*)(void *, int)) is a pain to write.
and exporting struct backend, even
opaquely, to the public header didn't sound like a good idea either.
(Are C structs always treated the same by name? That could cause a
problem for a filter which used "struct backend" for some internal
reason I think.)
None of our in-tree filters declare 'struct backend'. C allows two
different .o files to declare a struct by the same name but with
different layout, so you are right that adding an incomplete type in our
public header COULD interfere with a pre-existing filter that already
had an internal declaration of the same struct name. But since we don't
promise API stability for filters, and none of our in-tree filters would
break, I don't see it being a problem. I'll go ahead and propose the
followup patch that exposes the incomplete type in the public header,
and we can decide from there.
> But one step at a time; your patch is fine as-is.
> ACK
Thanks - pushed. Hopefully shouldn't break your new filter.
Nope, no interaction, so my ext2 code is now pushed too.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization:
qemu.org |
libvirt.org