On Wed, Jan 25, 2012 at 05:19:42PM +0000, Matthew Booth wrote:

This change binds guestfs_close(). It consequently results in RConstOptString being able to throw an error. --- generator/generator_gobject.ml | 79 ++++++++++++++++++++++++++++------------ gobject/run-bindtests | 1 + gobject/tests-misc.js | 41 +++++++++++++++++++++ 3 files changed, 97 insertions(+), 24 deletions(-) create mode 100644 gobject/tests-misc.js

diff --git a/generator/generator_gobject.ml b/generator/generator_gobject.ml index 2b88274..b005dfe 100644 --- a/generator/generator_gobject.ml +++ b/generator/generator_gobject.ml @@ -156,6 +156,7 @@ struct _GuestfsSessionClass

GType guestfs_session_get_type(void); GuestfsSession *guestfs_session_new(void); +gboolean guestfs_session_close(GuestfsSession *session, GError **err);

/* Guestfs::Tristate */ typedef enum @@ -304,6 +305,16 @@ let generate_gobject_c_static () = * images. */

+/* Error quark */ + +#define GUESTFS_ERROR guestfs_error_quark() + +static GQuark +guestfs_error_quark(void) +{ + return g_quark_from_static_string(\"guestfs\"); +} + #define GUESTFS_SESSION_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE ( \ (obj), \ GUESTFS_TYPE_SESSION, \ @@ -357,6 +368,29 @@ guestfs_session_new(void) return GUESTFS_SESSION(g_object_new(GUESTFS_TYPE_SESSION, NULL)); }

+/** + * guestfs_session_close: + * + * Close a libguestfs session. + * + * Returns: true on success, false on error + */ +gboolean +guestfs_session_close(GuestfsSession *session, GError **err) +{ + guestfs_h *g = session->priv->g; + + if (g == NULL) { + g_set_error_literal(err, GUESTFS_ERROR, 0, \"Session is already closed\");

Don't capitalize error strings.

+ return FALSE; + } + + guestfs_close(g); + session->priv->g = NULL; + + return TRUE; +} + /* Guestfs::Tristate */ GType guestfs_tristate_get_type(void) @@ -374,16 +408,6 @@ guestfs_tristate_get_type(void) return etype; }

-/* Error quark */ - -#define GUESTFS_ERROR guestfs_error_quark() - -static GQuark -guestfs_error_quark(void) -{ - return g_quark_from_static_string(\"guestfs\"); -} - /* Cancellation handler */ static void cancelled_handler(gpointer data) @@ -598,7 +622,15 @@ let generate_gobject_c_methods () = let doc = String.concat "\n * " doc in let camel_name = camel_of_name flags name in let is_RBufferOut = match ret with RBufferOut _ -> true | _ -> false in - let error_return = match ret with + let guestfs_error_return = match ret with + | RErr | RInt _ | RInt64 _ | RBool _ -> + "-1" + | RConstString _ | RString _ | RStringList _ | RHashtable _ + | RBufferOut _ | RStruct _ | RStructList _ -> + "NULL" + | RConstOptString _ -> "" (* We don't check RConstOptString for error *) + in

Why not use 'errcode_of_ret' instead of hard-coding this everywhere?

+ let gobject_error_return = match ret with | RErr -> "FALSE" | RInt _ | RInt64 _ | RBool _ -> @@ -606,7 +638,9 @@ let generate_gobject_c_methods () = | RConstString _ | RString _ | RStringList _ | RHashtable _ | RBufferOut _ | RStruct _ | RStructList _ -> "NULL" - | RConstOptString _ -> "" + | RConstOptString _ -> + "NULL" (* NULL is a valid return for RConstOptString. Error is + indicated by also setting *err to a non-NULL value *) in

(* The comment header, including GI annotations for arguments and the @@ -684,10 +718,16 @@ let generate_gobject_c_methods () = if cancellable then ( pr " /* Check we haven't already been cancelled */\n"; pr " if (g_cancellable_set_error_if_cancelled (cancellable, err))\n"; - pr " return %s;\n\n" error_return; + pr " return %s;\n\n" gobject_error_return; );

+ (* Get the guestfs handle, and ensure it isn't closed *) + pr " guestfs_h *g = session->priv->g;\n"; + pr " if (g == NULL) {\n"; + pr " g_set_error_literal(err, GUESTFS_ERROR, 0, \"Attempt to call %s after the session has been closed\");\n" name;

GCC combines common static strings together, reducing the final size of the .text segment. However this doesn't work if you generate strings which are all different. You should factor out the common part of strings, so: g_set_error (..., \"attempted to call %%s on closed handle\", \"%s\"); Also don't capitalize error messages ...

+ pr " return %s;\n" gobject_error_return; + pr " }\n\n";

(* Optargs *)

@@ -777,18 +817,9 @@ let generate_gobject_c_methods () = (* Check return, throw error if necessary, marshall return value *)

if match ret with RConstOptString _ -> false | _ -> true then ( - pr " if (ret == %s) {\n" - (match ret with - | RErr | RInt _ | RInt64 _ | RBool _ -> - "-1" - | RConstString _ | RString _ | RStringList _ | RHashtable _ - | RBufferOut _ | RStruct _ | RStructList _ -> - "NULL" - | RConstOptString _ -> - assert false; - ); + pr " if (ret == %s) {\n" guestfs_error_return; pr " g_set_error_literal(err, GUESTFS_ERROR, 0, guestfs_last_error(g));\n"; - pr " return %s;\n" error_return; + pr " return %s;\n" gobject_error_return; pr " }\n"; ); pr "\n"; diff --git a/gobject/run-bindtests b/gobject/run-bindtests index 55c489c..d2ce0a6 100755 --- a/gobject/run-bindtests +++ b/gobject/run-bindtests @@ -26,3 +26,4 @@ fi ../run $GJS $srcdir/bindtests.js > bindtests.tmp diff -u ${srcdir}/../bindtests bindtests.tmp ../run $GJS $srcdir/bindtests-manual.js 2>/dev/null +../run $GJS $srcdir/tests-misc.js 2>/dev/null diff --git a/gobject/tests-misc.js b/gobject/tests-misc.js new file mode 100644 index 0000000..aadc2f3 --- /dev/null +++ b/gobject/tests-misc.js @@ -0,0 +1,41 @@ +// libguestfs miscellaneous gobject binding tests +// Copyright (C) 2012 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. + +const Guestfs = imports.gi.Guestfs; + +var fail = false; + +var g = new Guestfs.Session(); + +// Test close() +g.close(); +var threw = false; +try { + var v = g.test0rconstoptstring('1'); +} catch (error) { + threw = true; + if (!error.message.match(/closed/)) { + print("call after close threw unexpected error: " + error.message); + fail = true; + } +} +if (!threw) { + print("call after closed failed to throw an error"); + fail = true; +} + +fail ? 1 : 0; -- 1.7.7.5

_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs

-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw

This change binds guestfs_close(). It consequently results in RConstOptString being able to throw an error. --- generator/generator_gobject.ml | 56 +++++++++++++++++++++++++++------- gobject/Makefile.am | 5 ++- gobject/{run-bindtests => run-tests} | 1 + gobject/tests-misc.js | 41 +++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 13 deletions(-) rename gobject/{run-bindtests => run-tests} (95%) create mode 100644 gobject/tests-misc.js diff --git a/generator/generator_gobject.ml b/generator/generator_gobject.ml index abc9062..7bac45e 100644 --- a/generator/generator_gobject.ml +++ b/generator/generator_gobject.ml @@ -156,6 +156,7 @@ struct _GuestfsSessionClass GType guestfs_session_get_type(void); GuestfsSession *guestfs_session_new(void); +gboolean guestfs_session_close(GuestfsSession *session, GError **err); /* Guestfs::Tristate */ typedef enum @@ -304,6 +305,16 @@ let generate_gobject_c_static () = * images. */ +/* Error quark */ + +#define GUESTFS_ERROR guestfs_error_quark() + +static GQuark +guestfs_error_quark(void) +{ + return g_quark_from_static_string(\"guestfs\"); +} + #define GUESTFS_SESSION_GET_PRIVATE(obj) (G_TYPE_INSTANCE_GET_PRIVATE ( \ (obj), \ GUESTFS_TYPE_SESSION, \ @@ -357,6 +368,29 @@ guestfs_session_new(void) return GUESTFS_SESSION(g_object_new(GUESTFS_TYPE_SESSION, NULL)); } +/** + * guestfs_session_close: + * + * Close a libguestfs session. + * + * Returns: true on success, false on error + */ +gboolean +guestfs_session_close(GuestfsSession *session, GError **err) +{ + guestfs_h *g = session->priv->g; + + if (g == NULL) { + g_set_error_literal(err, GUESTFS_ERROR, 0, \"session is already closed\"); + return FALSE; + } + + guestfs_close(g); + session->priv->g = NULL; + + return TRUE; +} + /* Guestfs::Tristate */ GType guestfs_tristate_get_type(void) @@ -374,16 +408,6 @@ guestfs_tristate_get_type(void) return etype; } -/* Error quark */ - -#define GUESTFS_ERROR guestfs_error_quark() - -static GQuark -guestfs_error_quark(void) -{ - return g_quark_from_static_string(\"guestfs\"); -} - /* Cancellation handler */ static void cancelled_handler(gpointer data) @@ -606,7 +630,9 @@ let generate_gobject_c_methods () = | RConstString _ | RString _ | RStringList _ | RHashtable _ | RBufferOut _ | RStruct _ | RStructList _ -> "NULL" - | RConstOptString _ -> "" + | RConstOptString _ -> + "NULL" (* NULL is a valid return for RConstOptString. Error is + indicated by also setting *err to a non-NULL value *) in (* The comment header, including GI annotations for arguments and the @@ -687,7 +713,15 @@ let generate_gobject_c_methods () = pr " return %s;\n\n" gobject_error_return; ); + (* Get the guestfs handle, and ensure it isn't closed *) + pr " guestfs_h *g = session->priv->g;\n"; + pr " if (g == NULL) {\n"; + pr " g_set_error(err, GUESTFS_ERROR, 0,\n"; + pr " \"attempt to call %%s after the session has been closed\",\n"; + pr " \"%s\");\n" name; + pr " return %s;\n" gobject_error_return; + pr " }\n\n"; (* Optargs *) diff --git a/gobject/Makefile.am b/gobject/Makefile.am index 5ea4488..71de7cc 100644 --- a/gobject/Makefile.am +++ b/gobject/Makefile.am @@ -22,7 +22,8 @@ BUILT_SOURCES = \ EXTRA_DIST = \ $(BUILT_SOURCES) \ bindtests-manual.js \ - run-bindtests + tests-misc.js \ + run-tests libname = libguestfs-gobject-1.0.la @@ -60,7 +61,7 @@ gir_DATA = $(INTROSPECTION_GIRS) typelibdir = $(libdir)/girepository-1.0 typelib_DATA = $(INTROSPECTION_GIRS:.gir=.typelib) -TESTS = run-bindtests +TESTS = run-tests TESTS_ENVIRONMENT = GJS=$(GJS) $(TESTS): $(typelib_DATA) diff --git a/gobject/run-bindtests b/gobject/run-tests similarity index 95% rename from gobject/run-bindtests rename to gobject/run-tests index 55c489c..d2ce0a6 100755 --- a/gobject/run-bindtests +++ b/gobject/run-tests @@ -26,3 +26,4 @@ fi ../run $GJS $srcdir/bindtests.js > bindtests.tmp diff -u ${srcdir}/../bindtests bindtests.tmp ../run $GJS $srcdir/bindtests-manual.js 2>/dev/null +../run $GJS $srcdir/tests-misc.js 2>/dev/null diff --git a/gobject/tests-misc.js b/gobject/tests-misc.js new file mode 100644 index 0000000..aadc2f3 --- /dev/null +++ b/gobject/tests-misc.js @@ -0,0 +1,41 @@ +// libguestfs miscellaneous gobject binding tests +// Copyright (C) 2012 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. + +const Guestfs = imports.gi.Guestfs; + +var fail = false; + +var g = new Guestfs.Session(); + +// Test close() +g.close(); +var threw = false; +try { + var v = g.test0rconstoptstring('1'); +} catch (error) { + threw = true; + if (!error.message.match(/closed/)) { + print("call after close threw unexpected error: " + error.message); + fail = true; + } +} +if (!threw) { + print("call after closed failed to throw an error"); + fail = true; +} + +fail ? 1 : 0; -- 1.7.7.5

On Thu, Jan 26, 2012 at 11:49:54AM +0000, Matthew Booth wrote:

RConstOptString cannot return an error in the C api. This makes it a special case for the GObject api, as all other return types have a corresponding GError **err argument to return an error. This change removes this special case, and includes the possibility of an error return in the API. An error is indicated by setting *err to a non-NULL value.

This change is in preparation for adding a close api. An attempt to call any api, even RConstOptString, on a closed handle must return an error. --- generator/generator_gobject.ml | 29 +++++++++++------------------ 1 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/generator/generator_gobject.ml b/generator/generator_gobject.ml index 4779f23..abc9062 100644 --- a/generator/generator_gobject.ml +++ b/generator/generator_gobject.ml @@ -37,10 +37,6 @@ let camel_of_name flags name = a ^ String.uppercase (Str.first_chars b 1) ^ Str.string_after b 1 ) "" (Str.split (regexp "_") name)

-let returns_error = function - | RConstOptString _ -> false - | _ -> true - let generate_gobject_proto name ?(single_line = true) (ret, args, optargs) flags = let spacer = if single_line then " " else "\n" in @@ -109,7 +105,7 @@ let generate_gobject_proto name ?(single_line = true) | _ -> ()); if List.exists (function Cancellable -> true | _ -> false) flags then pr ", GCancellable *cancellable"; - if returns_error ret then pr ", GError **err"; + pr ", GError **err"; pr ")"

let generate_gobject_header_static () = @@ -602,7 +598,7 @@ let generate_gobject_c_methods () = let doc = String.concat "\n * " doc in let camel_name = camel_of_name flags name in let is_RBufferOut = match ret with RBufferOut _ -> true | _ -> false in - let error_return = match ret with + let gobject_error_return = match ret with | RErr -> "FALSE" | RInt _ | RInt64 _ | RBool _ -> @@ -688,7 +684,7 @@ let generate_gobject_c_methods () = if cancellable then ( pr " /* Check we haven't already been cancelled */\n"; pr " if (g_cancellable_set_error_if_cancelled (cancellable, err))\n"; - pr " return %s;\n\n" error_return; + pr " return %s;\n\n" gobject_error_return; );

pr " guestfs_h *g = session->priv->g;\n"; @@ -780,19 +776,16 @@ let generate_gobject_c_methods () =

(* Check return, throw error if necessary, marshall return value *)

- if returns_error ret then ( + (match errcode_of_ret ret with + | `CannotReturnError -> () + | _ -> pr " if (ret == %s) {\n" - (match ret with - | RErr | RInt _ | RInt64 _ | RBool _ -> - "-1" - | RConstString _ | RString _ | RStringList _ | RHashtable _ - | RBufferOut _ | RStruct _ | RStructList _ -> - "NULL" - | RConstOptString _ -> - assert false; - ); + (match errcode_of_ret ret with + | `CannotReturnError -> assert false + | `ErrorIsMinusOne -> "-1" + | `ErrorIsNULL -> "NULL"); pr " g_set_error_literal(err, GUESTFS_ERROR, 0, guestfs_last_error(g));\n"; - pr " return %s;\n" error_return; + pr " return %s;\n" gobject_error_return; pr " }\n"; ); pr "\n";

ACK. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw