[Libguestfs] [PATCH v2 1/3] generator: Implement Pointer arguments.

Richard W.M. Jones rjones at redhat.com
Wed Dec 10 21:15:43 UTC 2014


This implements Pointer arguments properly, at least for certain
limited definitions of "implements" and "properly".

'Pointer' as an argument type is meant to indicate a pointer passed to
an API.  The canonical example is the following proposed API:

  int guestfs_add_libvirt_dom (guestfs_h *g, virDomainPtr dom, ...);

where 'dom' is described in the generator as:

  Pointer ("virDomainPtr", "dom")

Pointer existed already in the generator, but the implementation was
broken.  It is not used by any existing API.

There are two basic difficulties of implementing Pointer:

(1) In language bindings there is no portable way to turn (eg.) a Perl
Sys::Virt 'dom' object into a C virDomainPtr.

(2) We can't rely on <libvirt/libvirt.h> being included (since it's an
optional dependency).

In this commit, we solve (2) by using a 'void *'.

We don't solve (1), really.  Instead we have a macro
POINTER_NOT_IMPLEMENTED which is used by currently all the non-C
language bindings.  It complains loudly and passes a NULL to the
underlying function.  The underlying function detects the NULL and
safely returns an error.  It is to be hoped that people will
contribute patches to make each language binding work, although in
some bindings it will always remain impossible to implement.
---
 generator/actions.ml            | 11 +++++++++--
 generator/c.ml                  |  2 +-
 generator/erlang.ml             |  2 +-
 generator/gobject.ml            | 10 +++++-----
 generator/golang.ml             |  4 ++--
 generator/java.ml               |  4 ++--
 generator/lua.ml                |  5 +++--
 generator/ocaml.ml              |  2 +-
 generator/perl.ml               |  2 +-
 generator/php.ml                | 15 +++++++++++----
 generator/python.ml             |  4 ++--
 generator/ruby.ml               |  3 ++-
 src/guestfs-internal-frontend.h | 11 +++++++++++
 13 files changed, 51 insertions(+), 24 deletions(-)

diff --git a/generator/actions.ml b/generator/actions.ml
index ba97eb3..93e3d0d 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12400,9 +12400,16 @@ let is_documented { visibility = v } = match v with
   | VPublic | VStateTest -> true
   | VBindTest | VDebug | VInternal -> false
 
-let is_fish { visibility = v } = match v with
-  | VPublic | VDebug -> true
+let is_fish { visibility = v; style = (_, args, _) } =
+  (* Internal functions are not exported to guestfish. *)
+  match v with
   | VStateTest | VBindTest | VInternal -> false
+  | VPublic | VDebug ->
+    (* Functions that take Pointer parameters cannot be used in
+     * guestfish, since there is no way the user could safely
+     * generate a pointer.
+     *)
+    not (List.exists (function Pointer _ -> true | _ -> false) args)
 
 let external_functions =
   List.filter is_external all_functions
diff --git a/generator/c.ml b/generator/c.ml
index 9e17d32..541c843 100644
--- a/generator/c.ml
+++ b/generator/c.ml
@@ -148,7 +148,7 @@ let rec generate_prototype ?(extern = true) ?(static = false)
         pr "size_t %s_size" n
     | Pointer (t, n) ->
         next ();
-        pr "%s %s" t n
+        pr "void * /* really %s */ %s" t n
   ) args;
   if is_RBufferOut then (next (); pr "size_t *size_r");
   if optargs <> [] then (
diff --git a/generator/erlang.ml b/generator/erlang.ml
index f0c9c6a..35ddbe6 100644
--- a/generator/erlang.ml
+++ b/generator/erlang.ml
@@ -319,7 +319,7 @@ extern int64_t get_int64 (ETERM *term);
           | Int64 n ->
             pr "  int64_t %s = get_int64 (ARG (%d));\n" n i
           | Pointer (t, n) ->
-            assert false
+            pr "  void * /* %s */ %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n" t n t
       ) args;
 
       (* Optional arguments. *)
diff --git a/generator/gobject.ml b/generator/gobject.ml
index 499837a..3217b9c 100644
--- a/generator/gobject.ml
+++ b/generator/gobject.ml
@@ -89,8 +89,8 @@ let generate_gobject_proto name ?(single_line = true)
         pr "gchar *const *%s" n
       | BufferIn n ->
         pr "const guint8 *%s, gsize %s_size" n n
-      | Pointer _ ->
-        failwith "gobject bindings do not support Pointer arguments"
+      | Pointer (t, n) ->
+        pr "void * /* %s */ %s" t n
   ) args;
   if optargs <> [] then (
     pr ", %s *optargs" (camel_of_name f)
@@ -1056,7 +1056,7 @@ guestfs_session_close (GuestfsSession *session, GError **err)
             pr " (transfer none) (array length=%s_size) (element-type guint8): an array of binary data\n" n;
             pr " * @%s_size: The size of %s, in bytes" n n;
           | Pointer _ ->
-            failwith "gobject bindings do not support Pointer arguments"
+            pr "pointer (not implemented in gobject bindings)"
           );
           pr "\n";
       ) args;
@@ -1202,8 +1202,8 @@ guestfs_session_close (GuestfsSession *session, GError **err)
           | DeviceList n | Key n | FileIn n | FileOut n
           | GUID n ->
             pr "%s" n
-          | Pointer _ ->
-            failwith "gobject bindings do not support Pointer arguments"
+          | Pointer (_, n) ->
+            pr "%s" n
       ) args;
       if is_RBufferOut then pr ", size_r";
       if optargs <> [] then pr ", argvp";
diff --git a/generator/golang.ml b/generator/golang.ml
index e67ca1b..b4b2482 100644
--- a/generator/golang.ml
+++ b/generator/golang.ml
@@ -317,7 +317,7 @@ func return_hashtable (argv **C.char) map[string]string {
           | StringList n
           | DeviceList n -> pr "%s []string" n
           | BufferIn n -> pr "%s []byte" n
-          | Pointer _ -> assert false
+          | Pointer (_, n) -> pr "%s int64" n
       ) args;
       if optargs <> [] then (
         if !comma then pr ", ";
@@ -457,7 +457,7 @@ func return_hashtable (argv **C.char) map[string]string {
           | StringList n
           | DeviceList n -> pr "c_%s" n
           | BufferIn n -> pr "c_%s, C.size_t (len (%s))" n n
-          | Pointer _ -> assert false
+          | Pointer _ -> pr "nil"
       ) args;
       (match ret with
       | RBufferOut _ -> pr ", &size"
diff --git a/generator/java.ml b/generator/java.ml
index e9c5949..f763611 100644
--- a/generator/java.ml
+++ b/generator/java.ml
@@ -898,7 +898,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn)
         | Int64 n ->
             pr "  int64_t %s;\n" n
         | Pointer (t, n) ->
-            pr "  %s %s;\n" t n
+            pr "  void * /* %s */ %s;\n" t n
       ) args;
 
       if optargs <> [] then (
@@ -965,7 +965,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn)
         | Int64 n ->
             pr "  %s = j%s;\n" n n
         | Pointer (t, n) ->
-            pr "  %s = (%s) j%s;\n" n t n
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n" n t
       ) args;
 
       if optargs <> [] then (
diff --git a/generator/lua.ml b/generator/lua.ml
index 5d5619c..c1fa6f0 100644
--- a/generator/lua.ml
+++ b/generator/lua.ml
@@ -473,7 +473,7 @@ guestfs_lua_delete_event_callback (lua_State *L)
         | Bool n -> pr "  int %s;\n" n
         | Int n -> pr "  int %s;\n" n
         | Int64 n -> pr "  int64_t %s;\n" n
-        | Pointer (t, n) -> pr "  %s %s;\n" t n
+        | Pointer (t, n) -> pr "  void * /* %s */ %s;\n" t n
       ) args;
       if optargs <> [] then (
         pr "  struct %s optargs_s = { .bitmask = 0 };\n" c_function;
@@ -506,7 +506,8 @@ guestfs_lua_delete_event_callback (lua_State *L)
             pr "  %s = luaL_checkint (L, %d);\n" n i
           | Int64 n ->
             pr "  %s = get_int64 (L, %d);\n" n i
-          | Pointer (t, n) -> assert false
+          | Pointer (t, n) ->
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n" n t
       ) args;
 
       if optargs <> [] then (
diff --git a/generator/ocaml.ml b/generator/ocaml.ml
index bd27f7a..70237b9 100644
--- a/generator/ocaml.ml
+++ b/generator/ocaml.ml
@@ -539,7 +539,7 @@ copy_table (char * const * argv)
         | Int64 n ->
             pr "  int64_t %s = Int64_val (%sv);\n" n n
         | Pointer (t, n) ->
-            pr "  %s %s = (%s) (intptr_t) Int64_val (%sv);\n" t n t n
+            pr "  void * /* %s */ %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n" t n t
       ) args;
 
       (* Optional arguments. *)
diff --git a/generator/perl.ml b/generator/perl.ml
index 3da45fd..dcf746d 100644
--- a/generator/perl.ml
+++ b/generator/perl.ml
@@ -369,7 +369,7 @@ PREINIT:
           | Bool n -> pr "      int %s;\n" n
           | Int n -> pr "      int %s;\n" n
           | Int64 n -> pr "      int64_t %s;\n" n
-          | Pointer (t, n) -> pr "      %s %s;\n" t n
+          | Pointer (t, n) -> pr "      void * /* %s */ %s;\n" t n
       ) args;
 
       (* PREINIT section (local variable declarations). *)
diff --git a/generator/php.ml b/generator/php.ml
index c7e0a27..2f30485 100644
--- a/generator/php.ml
+++ b/generator/php.ml
@@ -87,6 +87,7 @@ and generate_php_c () =
 #include <php_guestfs_php.h>
 
 #include \"guestfs.h\"
+#include \"guestfs-internal-frontend.h\"
 
 static int res_guestfs_h;
 
@@ -202,8 +203,10 @@ PHP_FUNCTION (guestfs_last_error)
             pr "  char **%s;\n" n;
         | Bool n ->
             pr "  zend_bool %s;\n" n
-        | Int n | Int64 n | Pointer (_, n) ->
+        | Int n | Int64 n ->
             pr "  long %s;\n" n
+        | Pointer (t, n) ->
+            pr "  void * /* %s */ %s;\n" t n
         ) args;
 
       if optargs <> [] then (
@@ -241,7 +244,8 @@ PHP_FUNCTION (guestfs_last_error)
           | OptString n -> "s!"
           | StringList n | DeviceList n -> "a"
           | Bool n -> "b"
-          | Int n | Int64 n | Pointer (_, n) -> "l"
+          | Int n | Int64 n -> "l"
+          | Pointer _ -> ""
         ) args
       ) in
 
@@ -277,8 +281,9 @@ PHP_FUNCTION (guestfs_last_error)
             pr ", &z_%s" n
         | Bool n ->
             pr ", &%s" n
-        | Int n | Int64 n | Pointer (_, n) ->
+        | Int n | Int64 n ->
             pr ", &%s" n
+        | Pointer (_, n) -> ()
       ) args;
       List.iter (
         function
@@ -342,7 +347,9 @@ PHP_FUNCTION (guestfs_last_error)
             pr "    %s[c] = NULL;\n" n;
             pr "  }\n";
             pr "\n"
-        | Bool _ | Int _ | Int64 _ | Pointer _ -> ()
+        | Bool _ | Int _ | Int64 _ -> ()
+        | Pointer (t, n) ->
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n" n t
         ) args;
 
       (* Optional arguments. *)
diff --git a/generator/python.ml b/generator/python.ml
index 07e87d2..82afb2e 100644
--- a/generator/python.ml
+++ b/generator/python.ml
@@ -295,7 +295,7 @@ put_table (char * const * const argv)
         | Int64 n -> pr "  long long %s;\n" n
         | Pointer (t, n) ->
             pr "  long long %s_int64;\n" n;
-            pr "  %s %s;\n" t n
+            pr "  void * /* %s */ %s;\n" t n
       ) args;
 
       (* Fetch the optional arguments as objects, so we can detect
@@ -370,7 +370,7 @@ put_table (char * const * const argv)
             pr "  %s = get_string_list (py_%s);\n" n n;
             pr "  if (!%s) goto out;\n" n
         | Pointer (t, n) ->
-            pr "  %s = (%s) (intptr_t) %s_int64;\n" n t n
+            pr "  %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n" n t
       ) args;
 
       pr "\n";
diff --git a/generator/ruby.ml b/generator/ruby.ml
index 88762ca..a031681 100644
--- a/generator/ruby.ml
+++ b/generator/ruby.ml
@@ -612,7 +612,8 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn)
         | Int64 n ->
           pr "  long long %s = NUM2LL (%sv);\n" n n
         | Pointer (t, n) ->
-          pr "  %s %s = (%s) (intptr_t) NUM2LL (%sv);\n" t n t n
+          pr "  (void) %sv;\n" n;
+          pr "  void * /* %s */ %s = POINTER_NOT_IMPLEMENTED (\"%s\");\n" t n t
       ) args;
       pr "\n";
 
diff --git a/src/guestfs-internal-frontend.h b/src/guestfs-internal-frontend.h
index 5c5d957..acb7014 100644
--- a/src/guestfs-internal-frontend.h
+++ b/src/guestfs-internal-frontend.h
@@ -183,4 +183,15 @@ extern GUESTFS_DLL_PUBLIC int guestfs___add_libvirt_dom (guestfs_h *g, virDomain
     }                                                                   \
   } while (0)
 
+/* Not all language bindings know how to deal with Pointer arguments.
+ * Those that don't will use this macro which complains noisily and
+ * returns NULL.
+ */
+#define POINTER_NOT_IMPLEMENTED(type)                                   \
+  (                                                                     \
+   fprintf (stderr, "*** WARNING: this language binding does not support conversion of Pointer(%s), so the current function will always fail.  Patches to fix this should be sent to the libguestfs upstream mailing list.\n", \
+            type),                                                      \
+   NULL                                                                 \
+)
+
 #endif /* GUESTFS_INTERNAL_FRONTEND_H_ */
-- 
2.1.0




More information about the Libguestfs mailing list