Re: [Libguestfs] [PATCH nbdkit filters-v2 2/5] Introduce filters.

On 01/19/2018 09:23 AM, Richard W.M. Jones wrote: > Filters can be placed in front of plugins to modify their behaviour. > > This commit adds the <nbdkit-filter.h> header file, the manual page, > the ‘filterdir’ directory (like ‘plugindir’), the ‘filters/’ source > directory which will contain the actual filters, the ‘--filters’ > parameter, and the filters backend logic. > --- > include/Makefile.am | 4 +- > include/nbdkit-filter.h | 149 +++++++++++ > include/nbdkit-plugin.h | 2 + > +++ b/include/nbdkit-filter.h > + > +#ifndef NBDKIT_FILTER_H > +#define NBDKIT_FILTER_H > + > +/* This header also defines some useful functions like nbdkit_debug > + * and nbdkit_parse_size which are appropriate for filters to use. > + */ > +#include <nbdkit-plugin.h> Should we stick the useful functions in a new header <nbdkit-common.h> that both nbdkit-plugin.h and nbdkit-filter.h include? I'm worried that the games we play with NBDKIT_API_VERSION in nbdkit-plugin.h for maintaining back-compat there may someday have awkward interactions in this file.

> + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#define NBDKIT_FILTER_API_VERSION 1 On the other hand, you're setting NBDKIT_FILTER_API_VERSION separately from NBDKIT_API_VERSION (for plugins), as we could bump one but not the other in a given nbdkit release (and if so, it makes for our next release to have PLUGIN version 2 but FILTER version 1, once filters and FUA flags settle, where the highest version in each file shares the same backend semantics). Does it ever make sense for a single .so file to call both NBDKIT_REGISTER_PLUGIN() and NBDKIT_REGISTER_FILTER() in the same library?

If not, one benefit of having the common code in a separate nbdkit-common.h is that we could do things like: nbdkit-common.h: #if !defined NBDKIT_PLUGIN_H || !defined NBDKIT_FILTER_H #error this file should not be directly included #endif nbdkit-plugin.h: #ifdef NBDKIT_FILTER_H #error cannot mix filter and plugin in one code base #endif nbdkit-filter.h: #ifdef NBDKIT_PLUGIN_H #error cannot mix filter and plugin in one code base #endif > + > +#ifndef NBDKIT_CXX_LANG_C > +#ifdef __cplusplus > +#define NBDKIT_CXX_LANG_C extern "C" > +#else > +#define NBDKIT_CXX_LANG_C /* nothing */ > +#endif > +#endif and a common header would be a nicer place for things like NBDKIT_CXX_LANG_C, rather than having to repeat our #ifndef definitions in two files.