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)
2026-03-21 22:53:37 +00:00 · 2017-06-08 10:58:39 +01:00
parent 28c375701d
commit 746a87cbf8
13 changed files with 180 additions and 36 deletions
--- a/.gitignore
+++ b/.gitignore
@@ -123,6 +123,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
@@ -361,7 +363,6 @@ Makefile.in
 /mllib/JSON_tests
 /mllib/libdir.ml
 /mllib/oUnit-*
-/mllib/visit_tests
 /ocaml/bindtests.bc
 /ocaml/bindtests.opt
 /ocaml/bindtests.ml
--- 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 \
--- a/common/mlvisit/Makefile.am
+++ 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
--- a/common/mlvisit/dummy.c
+++ 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 };
--- a/common/mlvisit/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);
 }
--- a/common/mlvisit/visit.ml
+++ b/common/mlvisit/visit.ml
--- a/common/mlvisit/visit.mli
+++ b/common/mlvisit/visit.mli
--- a/common/mlvisit/visit_tests.ml
+++ b/common/mlvisit/visit_tests.ml
--- a/configure.ac
+++ b/configure.ac
@@ -196,6 +196,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
--- 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
--- 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.
--- 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

--- 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) \