12:29 p.m.
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 7ccc928bb..63e0e0778 100644
--- 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
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