gerror: Add support for marshaling GError from Python to C

Refactor pygi_gerror_exception_check() to use a new broken out function
pygi_error_marshal_from_py(). This allows re-use for argument marshaling
of a Python GError to a C GError.
Remove PYGI_META_ARG_TYPE_CHILD setting for GError out argument marshaling.
This was incorrect since GError exception arguments are not specified
explicitly and instead uses the "throws" option.

https://bugzilla.gnome.org/show_bug.cgi?id=685197

3 files changed, 123 insertions, 42 deletions

diff --git a/gi/pygi-error.c b/gi/pygi-error.c
index 725c7c6d..9a9a10d4 100644
--- a/gi/pygi-error.c
+++ b/gi/pygi-error.c
@@ -108,6 +108,64 @@ pygi_error_check (GError **error)
 }
 
 /**
+ * pygi_error_marshal_from_py:
+ * @pyerr: A Python exception instance.
+ * @error: a standard GLib GError ** output parameter
+ *
+ * Converts from a Python implemented GError into a GError.
+ *
+ * Returns: TRUE if the conversion was successful, otherwise a Python exception
+ *          is set and FALSE is returned.
+ */
+gboolean
+pygi_error_marshal_from_py (PyObject *pyerr, GError **error)
+{
+    gboolean res = FALSE;
+    PyObject *py_message = NULL,
+             *py_domain = NULL,
+             *py_code = NULL;
+
+    if (PyObject_IsInstance (pyerr, PyGError) != 1) {
+        PyErr_Format (PyExc_TypeError, "Must be GLib.Error, not %s",
+                      pyerr->ob_type->tp_name);
+        return FALSE;
+    }
+
+    py_message = PyObject_GetAttrString (pyerr, "message");
+    if (!py_message || !PYGLIB_PyUnicode_Check (py_message)) {
+        PyErr_SetString (PyExc_ValueError,
+                         "GLib.Error instances must have a 'message' string attribute");
+        goto cleanup;
+    }
+
+    py_domain = PyObject_GetAttrString (pyerr, "domain");
+    if (!py_domain || !PYGLIB_PyUnicode_Check (py_domain)) {
+        PyErr_SetString (PyExc_ValueError,
+                         "GLib.Error instances must have a 'domain' string attribute");
+        goto cleanup;
+    }
+
+    py_code = PyObject_GetAttrString (pyerr, "code");
+    if (!py_code || !PYGLIB_PyLong_Check (py_code)) {
+        PyErr_SetString (PyExc_ValueError,
+                         "GLib.Error instances must have a 'code' int attribute");
+        goto cleanup;
+    }
+
+    res = TRUE;
+    g_set_error_literal (error,
+                         g_quark_from_string (PYGLIB_PyUnicode_AsString (py_domain)),
+                         PYGLIB_PyLong_AsLong (py_code),
+                         PYGLIB_PyUnicode_AsString (py_message));
+
+cleanup:
+    Py_XDECREF (py_message);
+    Py_XDECREF (py_code);
+    Py_XDECREF (py_domain);
+    return res;
+}
+
+/**
  * pygi_gerror_exception_check:
  * @error: a standard GLib GError ** output parameter
  *
@@ -121,10 +179,8 @@ pygi_error_check (GError **error)
 gboolean
 pygi_gerror_exception_check (GError **error)
 {
+    int res = -1;
     PyObject *type, *value, *traceback;
-    PyObject *py_message, *py_domain, *py_code;
-    const char *bad_gerror_message;
-
     PyErr_Fetch(&type, &value, &traceback);
     if (type == NULL)
         return 0;
@@ -144,44 +200,14 @@ pygi_gerror_exception_check (GError **error)
     Py_DECREF(type);
     Py_XDECREF(traceback);
 
-    py_message = PyObject_GetAttrString(value, "message");
-    if (!py_message || !PYGLIB_PyUnicode_Check(py_message)) {
-        bad_gerror_message = "GLib.Error instances must have a 'message' string attribute";
-        Py_XDECREF(py_message);
-        goto bad_gerror;
-    }
-
-    py_domain = PyObject_GetAttrString(value, "domain");
-    if (!py_domain || !PYGLIB_PyUnicode_Check(py_domain)) {
-        bad_gerror_message = "GLib.Error instances must have a 'domain' string attribute";
-        Py_DECREF(py_message);
-        Py_XDECREF(py_domain);
-        goto bad_gerror;
-    }
-
-    py_code = PyObject_GetAttrString(value, "code");
-    if (!py_code || !PYGLIB_PyLong_Check(py_code)) {
-        bad_gerror_message = "GLib.Error instances must have a 'code' int attribute";
-        Py_DECREF(py_message);
-        Py_DECREF(py_domain);
-        Py_XDECREF(py_code);
-        goto bad_gerror;
+    if (!pygi_error_marshal_from_py (value, error)) {
+        PyErr_Print();
+        res = -2;
     }
 
-    g_set_error(error, g_quark_from_string(PYGLIB_PyUnicode_AsString(py_domain)),
-                PYGLIB_PyLong_AsLong(py_code), "%s", PYGLIB_PyUnicode_AsString(py_message));
-
-    Py_DECREF(py_message);
-    Py_DECREF(py_code);
-    Py_DECREF(py_domain);
-    return -1;
-
-bad_gerror:
     Py_DECREF(value);
-    g_set_error(error, g_quark_from_static_string("pygi"), 0, "%s", bad_gerror_message);
-    PyErr_SetString(PyExc_ValueError, bad_gerror_message);
-    PyErr_Print();
-    return -2;
+    return res;
+
 }
 
 /**
@@ -221,9 +247,27 @@ _pygi_marshal_from_py_gerror (PyGIInvokeState   *state,
                               GIArgument        *arg,
                               gpointer          *cleanup_data)
 {
-    PyErr_Format (PyExc_NotImplementedError,
-                  "Marshalling for GErrors is not implemented");
-    return FALSE;
+    GError *error = NULL;
+    if (pygi_error_marshal_from_py (py_arg, &error)) {
+        arg->v_pointer = error;
+        *cleanup_data = error;
+        return TRUE;
+    } else {
+        return FALSE;
+    }
+}
+
+
+static void
+_pygi_marshal_from_py_gerror_cleanup  (PyGIInvokeState *state,
+                                       PyGIArgCache    *arg_cache,
+                                       PyObject        *py_arg,
+                                       gpointer         data,
+                                       gboolean         was_processed)
+{
+    if (was_processed) {
+        g_error_free ((GError *)data);
+    }
 }
 
 static PyObject *
@@ -261,7 +305,11 @@ pygi_arg_gerror_setup_from_info (PyGIArgCache  *arg_cache,
 
     if (direction & PYGI_DIRECTION_FROM_PYTHON) {
         arg_cache->from_py_marshaller = _pygi_marshal_from_py_gerror;
-        arg_cache->meta_type = PYGI_META_ARG_TYPE_CHILD;
+
+        /* Assign cleanup function if we manage memory after call completion. */
+        if (arg_cache->transfer == GI_TRANSFER_NOTHING) {
+            arg_cache->from_py_cleanup = _pygi_marshal_from_py_gerror_cleanup;
+        }
     }
 
     if (direction & PYGI_DIRECTION_TO_PYTHON) {
diff --git a/gi/pygi-error.h b/gi/pygi-error.h
index 2ef62f92..378c1b42 100644
--- a/gi/pygi-error.h
+++ b/gi/pygi-error.h
@@ -31,6 +31,9 @@ gboolean      pygi_error_check              (GError **error);
 
 PyObject*     pygi_error_marshal_to_py      (GError **error);
 
+gboolean      pygi_error_marshal_from_py    (PyObject  *pyerr,
+                                             GError   **error);
+
 gboolean      pygi_gerror_exception_check   (GError **error);
 
 PyObject*     pygi_register_exception_for_domain (gchar *name,
diff --git a/tests/test_overrides_glib.py b/tests/test_overrides_glib.py
index 4f630dc4..891923eb 100644
--- a/tests/test_overrides_glib.py
+++ b/tests/test_overrides_glib.py
@@ -495,6 +495,36 @@ class TestGVariant(unittest.TestCase):
         v = GLib.Variant('(is)', (1, 'somestring'))
         self.assertEqual(str(v), "(1, 'somestring')")
 
+    def test_parse_error(self):
+        # This test doubles as a test for GLib.Error marshaling.
+        source_str = 'abc'
+        with self.assertRaises(GLib.Error) as context:
+            GLib.Variant.parse(None, source_str, None, None)
+        e = context.exception
+        text = GLib.Variant.parse_error_print_context(e, source_str)
+        self.assertTrue(source_str in text)
+
+    def test_parse_error_exceptions(self):
+        source_str = 'abc'
+        self.assertRaisesRegexp(TypeError, 'Must be GLib.Error, not int',
+                                GLib.Variant.parse_error_print_context,
+                                42, source_str)
+
+        gerror = GLib.Error(message=42)  # not a string
+        self.assertRaisesRegexp(ValueError, ".*must have a 'message'.*",
+                                GLib.Variant.parse_error_print_context,
+                                gerror, source_str)
+
+        gerror = GLib.Error(domain=42)  # not a string
+        self.assertRaisesRegexp(ValueError, ".*must have a 'domain'.*",
+                                GLib.Variant.parse_error_print_context,
+                                gerror, source_str)
+
+        gerror = GLib.Error(code='not an int')
+        self.assertRaisesRegexp(ValueError, ".*must have a 'code' int.*",
+                                GLib.Variant.parse_error_print_context,
+                                gerror, source_str)
+
 
 class TestConstants(unittest.TestCase):