[Libguestfs] [PATCH v6 01/41] mllib: Move Visit OCaml bindings to common/mlvisit.

The ‘Visit’ module is a self-contained library with the only dependencies being: - the C ‘visit’ implementation - the guestfs OCaml bindings Move it to a separate ‘common/mlvisit’ directory. This change is not entirely refactoring. Two other fixes are made: - remove unsafe use of CLEANUP_FREE from a function which could raise an OCaml exception (cleanup handlers would not be called correctly if the exception is thrown) - don't link directly to common/visit/visit.c, but instead use the library (common/visit/libvisit.la) --- .gitignore | 3 +- Makefile.am | 5 +- common/mlvisit/Makefile.am | 152 +++++++++++++++++++++++++++++++ common/mlvisit/dummy.c | 2 + {mllib => common/mlvisit}/visit-c.c | 6 +- {mllib => common/mlvisit}/visit.ml | 0 {mllib => common/mlvisit}/visit.mli | 0 {mllib => common/mlvisit}/visit_tests.ml | 0 configure.ac | 1 + docs/C_SOURCE_FILES | 3 +- docs/guestfs-hacking.pod | 4 + mllib/Makefile.am | 30 +----- sysprep/Makefile.am | 10 +- 13 files changed, 180 insertions(+), 36 deletions(-) diff --git a/.gitignore b/.gitignore index 69e1ae160..2367cddcb 100644 --- a/.gitignore +++ b/.gitignore @@ -124,6 +124,8 @@ Makefile.in /common/errnostring/errnostring-gperf.gperf /common/errnostring/errnostring.h /common/miniexpect/miniexpect.3 +/common/mlvisit/.depend +/common/mlvisit/visit_tests /common/protocol/guestfs_protocol.c /common/protocol/guestfs_protocol.h /common/protocol/guestfs_protocol.x @@ -366,7 +368,6 @@ Makefile.in /mllib/JSON_tests /mllib/libdir.ml /mllib/oUnit-* -/mllib/visit_tests /ocaml/bindtests.bc /ocaml/bindtests.opt /ocaml/bindtests.ml diff --git a/Makefile.am b/Makefile.am index ae77cdda2..499a1d279 100644 --- a/Makefile.am +++ b/Makefile.am @@ -150,10 +150,11 @@ endif # Unconditional because nothing is built yet. SUBDIRS += csharp -# OCaml tools. Note 'mllib' and 'customize' contain shared code used -# by other OCaml tools, so these must come first. +# OCaml tools. Note 'common/ml*', 'mllib' and 'customize' contain +# shared code used by other OCaml tools, so these must come first. if HAVE_OCAML SUBDIRS += \ + common/mlvisit \ mllib \ customize \ builder builder/templates \ diff --git a/common/mlvisit/Makefile.am b/common/mlvisit/Makefile.am new file mode 100644 index 000000000..51cbd2de6 --- /dev/null +++ b/common/mlvisit/Makefile.am @@ -0,0 +1,152 @@ +# libguestfs OCaml tools common code +# Copyright (C) 2011-2017 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +include $(top_srcdir)/subdir-rules.mk + +EXTRA_DIST = \ + $(SOURCES_MLI) \ + $(SOURCES_ML) \ + $(SOURCES_C) \ + visit_tests.ml + +SOURCES_MLI = \ + visit.mli + +SOURCES_ML = \ + visit.ml + +SOURCES_C = \ + visit-c.c + +if HAVE_OCAML + +# We pretend that we're building a C library. automake handles the +# compilation of the C sources for us. At the end we take the C +# objects and OCaml objects and link them into the OCaml library. +# This C library is never used. + +noinst_LIBRARIES = libmlvisit.a + +if !HAVE_OCAMLOPT +MLVISIT_CMA = mlvisit.cma +else +MLVISIT_CMA = mlvisit.cmxa +endif + +noinst_DATA = $(MLVISIT_CMA) + +libmlvisit_a_SOURCES = $(SOURCES_C) +libmlvisit_a_CPPFLAGS = \ + -I. \ + -I$(top_builddir) \ + -I$(top_srcdir)/gnulib/lib -I$(top_builddir)/gnulib/lib \ + -I$(shell $(OCAMLC) -where) \ + -I$(top_srcdir)/lib \ + -I$(top_srcdir)/common/visit +libmlvisit_a_CFLAGS = \ + $(WARN_CFLAGS) $(WERROR_CFLAGS) \ + $(LIBVIRT_CFLAGS) $(LIBXML2_CFLAGS) \ + -fPIC + +BOBJECTS = $(SOURCES_ML:.ml=.cmo) +XOBJECTS = $(BOBJECTS:.cmo=.cmx) + +# -I $(top_builddir)/lib/.libs is a hack which forces corresponding -L +# option to be passed to gcc, so we don't try linking against an +# installed copy of libguestfs. +OCAMLPACKAGES = \ + -package str,unix \ + -I $(top_builddir)/lib/.libs \ + -I $(top_builddir)/gnulib/lib/.libs \ + -I $(top_builddir)/ocaml \ + -I $(top_builddir)/common/utils/.libs \ + -I $(top_builddir)/common/visit/.libs \ + -I $(builddir) +OCAMLPACKAGES_TESTS = $(MLVISIT_CMA) + +OCAMLFLAGS = $(OCAML_FLAGS) $(OCAML_WARN_ERROR) + +if !HAVE_OCAMLOPT +OBJECTS = $(BOBJECTS) +else +OBJECTS = $(XOBJECTS) +endif + +libmlvisit_a_DEPENDENCIES = $(OBJECTS) + +$(MLVISIT_CMA): $(OBJECTS) libmlvisit.a + $(OCAMLFIND) mklib $(OCAMLPACKAGES) \ + $(OBJECTS) $(libmlvisit_a_OBJECTS) -cclib -lvisit -o mlvisit + +# Tests. + +visit_tests_SOURCES = dummy.c +visit_tests_BOBJECTS = visit_tests.cmo +visit_tests_XOBJECTS = $(visit_tests_BOBJECTS:.cmo=.cmx) + +# Can't call the following as <test>_OBJECTS because automake gets confused. +if !HAVE_OCAMLOPT +visit_tests_THEOBJECTS = $(visit_tests_BOBJECTS) +visit_tests.cmo: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS) +else +visit_tests_THEOBJECTS = $(visit_tests_XOBJECTS) +visit_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS) +endif + +OCAMLLINKFLAGS = mlguestfs.$(MLARCHIVE) $(LINK_CUSTOM_OCAMLC_ONLY) + +visit_tests_DEPENDENCIES = \ + $(visit_tests_THEOBJECTS) \ + $(MLVISIT_CMA) \ + $(top_srcdir)/ocaml-link.sh +visit_tests_LINK = \ + $(top_srcdir)/ocaml-link.sh \ + -cclib '-lvisit -lutils $(LIBXML2_LIBS) -lgnu' -- \ + $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ + $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ + $(visit_tests_THEOBJECTS) -o $@ + +TESTS_ENVIRONMENT = $(top_builddir)/run --test + +check_PROGRAMS = +TESTS = + +if ENABLE_APPLIANCE +check_PROGRAMS += visit_tests +TESTS += visit_tests +endif + +check-valgrind: + $(MAKE) VG="@VG@" check + +# Dependencies. +depend: .depend + +.depend: $(wildcard $(abs_srcdir)/*.mli) $(wildcard $(abs_srcdir)/*.ml) + rm -f $@ $@-t + $(OCAMLFIND) ocamldep -I ../../ocaml -I $(abs_srcdir) $^ | \ + $(SED) 's/ *$$//' | \ + $(SED) -e :a -e '/ *\\$$/N; s/ *\\\n */ /; ta' | \ + $(SED) -e 's,$(abs_srcdir)/,$(builddir)/,g' | \ + sort > $@-t + mv $@-t $@ + +-include .depend + +endif + +.PHONY: depend docs diff --git a/common/mlvisit/dummy.c b/common/mlvisit/dummy.c new file mode 100644 index 000000000..ebab6198c --- /dev/null +++ b/common/mlvisit/dummy.c @@ -0,0 +1,2 @@ +/* Dummy source, to be used for OCaml-based tools with no C sources. */ +enum { foo = 1 }; diff --git a/mllib/visit-c.c b/common/mlvisit/visit-c.c similarity index 98% rename from mllib/visit-c.c rename to common/mlvisit/visit-c.c index b1c12166c..4cd1c6cea 100644 --- a/mllib/visit-c.c +++ b/common/mlvisit/visit-c.c @@ -31,7 +31,6 @@ #include <caml/mlvalues.h> #include "guestfs.h" -#include "guestfs-internal.h" #include "visit.h" #pragma GCC diagnostic ignored "-Wmissing-prototypes" @@ -59,7 +58,7 @@ guestfs_int_mllib_visit (value gv, value dirv, value fv) /* The dir string could move around when we call the * visitor_function, so we have to take a full copy of it. */ - CLEANUP_FREE char *dir = strdup (String_val (dirv)); + char *dir = strdup (String_val (dirv)); /* This stack address is used to point to the exception, if one is * raised in the visitor_function. */ @@ -71,6 +70,8 @@ guestfs_int_mllib_visit (value gv, value dirv, value fv) args.fvp = &fv; if (visit (g, dir, visitor_function_wrapper, &args) == -1) { + free (dir); + if (exn != Val_unit) { /* The failure was caused by visitor_function raising an * exception. Re-raise it here. @@ -84,6 +85,7 @@ guestfs_int_mllib_visit (value gv, value dirv, value fv) */ caml_failwith ("visit"); } + free (dir); CAMLreturn (Val_unit); } diff --git a/mllib/visit.ml b/common/mlvisit/visit.ml similarity index 100% rename from mllib/visit.ml rename to common/mlvisit/visit.ml diff --git a/mllib/visit.mli b/common/mlvisit/visit.mli similarity index 100% rename from mllib/visit.mli rename to common/mlvisit/visit.mli diff --git a/mllib/visit_tests.ml b/common/mlvisit/visit_tests.ml similarity index 100% rename from mllib/visit_tests.ml rename to common/mlvisit/visit_tests.ml diff --git a/configure.ac b/configure.ac index cbb5101fc..4fc226123 100644 --- a/configure.ac +++ b/configure.ac @@ -185,6 +185,7 @@ AC_CONFIG_FILES([Makefile common/errnostring/Makefile common/edit/Makefile common/miniexpect/Makefile + common/mlvisit/Makefile common/options/Makefile common/parallel/Makefile common/progress/Makefile diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES index 15abec124..578126403 100644 --- a/docs/C_SOURCE_FILES +++ b/docs/C_SOURCE_FILES @@ -15,6 +15,8 @@ common/edit/file-edit.c common/edit/file-edit.h common/miniexpect/miniexpect.c common/miniexpect/miniexpect.h +common/mlvisit/dummy.c +common/mlvisit/visit-c.c common/options/config.c common/options/decrypt.c common/options/display-options.c @@ -340,7 +342,6 @@ mllib/getopt-c.c mllib/progress-c.c mllib/unix_utils-c.c mllib/uri-c.c -mllib/visit-c.c mllib/xml-c.c ocaml/guestfs-c-actions.c ocaml/guestfs-c-errnos.c diff --git a/docs/guestfs-hacking.pod b/docs/guestfs-hacking.pod index d3621c7e6..ac6d1ccbf 100644 --- a/docs/guestfs-hacking.pod +++ b/docs/guestfs-hacking.pod @@ -100,6 +100,10 @@ A copy of the miniexpect library from L<http://git.annexia.org/?p=miniexpect.git;a=summary>;. This is used in virt-p2v. +=item F<common/mlvisit> + +OCaml bindings for the visit functions (see F<common/visit>). + =item F<common/options> Common options parsing for guestfish, guestmount and some virt tools. diff --git a/mllib/Makefile.am b/mllib/Makefile.am index ee2f1a7a8..cb79f50f5 100644 --- a/mllib/Makefile.am +++ b/mllib/Makefile.am @@ -24,7 +24,6 @@ EXTRA_DIST = \ common_utils_tests.ml \ getopt_tests.ml \ JSON_tests.ml \ - visit_tests.ml \ test-getopt.sh SOURCES_MLI = \ @@ -41,8 +40,7 @@ SOURCES_MLI = \ regedit.mli \ registry.mli \ stringMap.mli \ - URI.mli \ - visit.mli + URI.mli SOURCES_ML = \ guestfs_config.ml \ @@ -55,7 +53,6 @@ SOURCES_ML = \ common_utils.ml \ progress.ml \ URI.ml \ - visit.ml \ planner.ml \ registry.ml \ regedit.ml \ @@ -66,7 +63,6 @@ SOURCES_ML = \ xpath_helpers.ml SOURCES_C = \ - ../common/visit/visit.c \ ../common/options/decrypt.c \ ../common/options/keys.c \ ../common/options/uri.c \ @@ -76,7 +72,6 @@ SOURCES_C = \ progress-c.c \ unix_utils-c.c \ uri-c.c \ - visit-c.c \ xml-c.c if HAVE_OCAML @@ -104,7 +99,6 @@ libmllib_a_CPPFLAGS = \ -I$(shell $(OCAMLC) -where) \ -I$(top_srcdir)/common/utils \ -I$(top_srcdir)/lib \ - -I$(top_srcdir)/common/visit \ -I$(top_srcdir)/common/options \ -I$(top_srcdir)/common/progress libmllib_a_CFLAGS = \ @@ -187,10 +181,6 @@ JSON_tests_SOURCES = dummy.c JSON_tests_BOBJECTS = JSON_tests.cmo JSON_tests_XOBJECTS = $(JSON_tests_BOBJECTS:.cmo=.cmx) -visit_tests_SOURCES = dummy.c -visit_tests_BOBJECTS = visit_tests.cmo -visit_tests_XOBJECTS = $(visit_tests_BOBJECTS:.cmo=.cmx) - # Can't call the following as <test>_OBJECTS because automake gets confused. if !HAVE_OCAMLOPT common_utils_tests_THEOBJECTS = $(common_utils_tests_BOBJECTS) @@ -210,9 +200,6 @@ getopt_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS) JSON_tests_THEOBJECTS = $(JSON_tests_XOBJECTS) JSON_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS) - -visit_tests_THEOBJECTS = $(visit_tests_XOBJECTS) -visit_tests.cmx: OCAMLPACKAGES += $(OCAMLPACKAGES_TESTS) endif OCAMLLINKFLAGS = mlguestfs.$(MLARCHIVE) $(LINK_CUSTOM_OCAMLC_ONLY) @@ -247,16 +234,6 @@ JSON_tests_LINK = \ $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ $(JSON_tests_THEOBJECTS) -o $@ -visit_tests_DEPENDENCIES = \ - $(visit_tests_THEOBJECTS) \ - $(MLLIB_CMA) \ - $(top_srcdir)/ocaml-link.sh -visit_tests_LINK = \ - $(top_srcdir)/ocaml-link.sh -cclib '-lutils $(LIBXML2_LIBS) -lgnu' -- \ - $(OCAMLFIND) $(BEST) $(OCAMLFLAGS) $(OCAMLLINKFLAGS) \ - $(OCAMLPACKAGES) $(OCAMLPACKAGES_TESTS) \ - $(visit_tests_THEOBJECTS) -o $@ - TESTS_ENVIRONMENT = $(top_builddir)/run --test TESTS = \ @@ -269,11 +246,6 @@ check_PROGRAMS += common_utils_tests JSON_tests TESTS += common_utils_tests JSON_tests endif -if ENABLE_APPLIANCE -check_PROGRAMS += visit_tests -TESTS += visit_tests -endif - check-valgrind: $(MAKE) VG="@VG@" check diff --git a/sysprep/Makefile.am b/sysprep/Makefile.am index 2a1ca25fd..68cb1814a 100644 --- a/sysprep/Makefile.am +++ b/sysprep/Makefile.am @@ -111,6 +111,8 @@ OCAMLPACKAGES = \ -I $(top_builddir)/lib/.libs \ -I $(top_builddir)/gnulib/lib/.libs \ -I $(top_builddir)/ocaml \ + -I $(top_builddir)/common/visit/.libs \ + -I $(top_builddir)/common/mlvisit \ -I $(top_builddir)/mllib \ -I $(top_builddir)/customize if HAVE_OCAML_PKG_GETTEXT @@ -118,6 +120,7 @@ OCAMLPACKAGES += -package gettext-stub endif OCAMLCLIBS = \ + -lvisit \ -lutils \ $(LIBTINFO_LIBS) \ $(LIBCRYPT_LIBS) \ @@ -133,7 +136,12 @@ else OBJECTS = $(XOBJECTS) endif -OCAMLLINKFLAGS = mlguestfs.$(MLARCHIVE) mllib.$(MLARCHIVE) customize.$(MLARCHIVE) $(LINK_CUSTOM_OCAMLC_ONLY) +OCAMLLINKFLAGS = \ + mlguestfs.$(MLARCHIVE) \ + mllib.$(MLARCHIVE) \ + mlvisit.$(MLARCHIVE) \ + customize.$(MLARCHIVE) \ + $(LINK_CUSTOM_OCAMLC_ONLY) virt_sysprep_DEPENDENCIES = \ $(OBJECTS) \ -- 2.13.0