[Libguestfs] [PATCH 2/2] gobject: Add an explicit close call
Richard W.M. Jones
rjones at redhat.com
Thu Jan 26 08:53:11 UTC 2012
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 at 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
More information about the Libguestfs
mailing list