Re: [Libguestfs] [PATCH nbdkit 2/2] tests: Test that public headers are ANSI (ISO C90) compatible.

On 1/14/19 6:15 AM, Richard W.M. Jones wrote: > In commit 3775916120749e935d287f35cb29ff6c586656df (and before that in > discussions with Eric) we decided that we should try to allow plugins > to be compiled with C compilers other than GCC or Clang (or with very > old / peculiar / incompatible versions of those compilers). > > However until we test this we cannot guarantee that changes to the > code will not break this in future, hence this test. > > Note that GCC or Clang is still required to compile nbdkit itself. > --- > tests/test-ansi-c-plugin.c | 167 +++++++++++++++++++++++++++++++++++++ > tests/Makefile.am | 24 ++++++ > tests/test-ansi-c.sh | 39 +++++++++ > 3 files changed, 230 insertions(+) > > diff --git a/tests/test-ansi-c-plugin.c b/tests/test-ansi-c-plugin.c > new file mode 100644 > index 0000000..8596ca5 > --- /dev/null > +++ b/tests/test-ansi-c-plugin.c > + > +#include <config.h> Do we really want to be including <config.h> in this file? This is the one case where we know we are standalone; I'm also worried that the macros defined in config.h were determined based on the existence of extensions during configure probes, and may be inaccurately-defined for compilation under a different set of compiler flags. I'd omit this (but perhaps leave a comment why), as proof that we are truly independent of how nbdkit was configured.

> +/* Strictly speaking (and we're compiling with -pedantic, so we _are_ lol > + * strictly speaking), ANSI C did not allow struct initialization > + * using labels. However it was a common extension to C compilers of > + * the period. > + * > + * Therefore we rely here on the order of fields in struct > + * nbdkit_plugin. But that's OK because of nbdkit's stable ABI > + * guarantee. > + * > + * If you are really writing an nbdkit plugin which can only use C90 > + * then you are advised to find the extension to your compiler which > + * allows you to initialize named fields. > + */ > +static struct nbdkit_plugin plugin = { > + 0, 0, 0, I'd typically use NULL for pointers, but using 0 is strictly portable, so I'm fine with it. > + "ansic", > + 0, > + PACKAGE_VERSION, > + 0, > + ansi_c_load, > + 0, > + 0, 0, 0, > + ansi_c_open, > + 0, > + ansi_c_get_size, > + 0, 0, 0, 0, > + ansi_c_pread > +}; > + > +NBDKIT_REGISTER_PLUGIN(plugin) > +++ b/tests/Makefile.am > > +# This builds a plugin using an ANSI (ISO C90) compiler to ensure that > +# the header file is compatible. The plugin does nothing very > +# interesting, it's mainly a compile test. > +TESTS += \ > + test-ansi-c.sh > +# check_LTLIBRARIES won't build a shared library (see automake manual). > +# So we have to do this and add a dependency. > +noinst_LTLIBRARIES += test-ansi-c-plugin.la > +test-ansi-c.sh: test-ansi-c-plugin.la > + > +test_ansi_c_plugin_la_SOURCES = \ > + test-ansi-c-plugin.c \ > + $(top_srcdir)/include/nbdkit-plugin.h > +test_ansi_c_plugin_la_CPPFLAGS = \ > + -std=c90 -pedantic \ > + -I$(top_srcdir)/include > +test_ansi_c_plugin_la_CFLAGS = \ > + $(WARNINGS_CFLAGS) > +# For use of the -rpath option, see: > +# https://lists.gnu.org/archive/html/libtool/2007-07/msg00067.html > +test_ansi_c_plugin_la_LDFLAGS = \ > + -module -avoid-version -shared -rpath /nowhere Looks sane, if it still works without <config.h>. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org