From 26c015b177ddcc0f35c97bcd7a4f2114fb2e8e2a Mon Sep 17 00:00:00 2001 From: Christoph Reiter Date: Sun, 29 Mar 2015 23:23:09 +0200 Subject: [PATCH] Field setters: Remove unneeded type/range checks and resulting unused code. These checks are performed in the actual marshalling code paths as well, no need to do them twice. Also move _pygi_g_registered_type_info_check_object() to pygi-info.c as it's the only place where it is still used. https://bugzilla.gnome.org/show_bug.cgi?id=746985 --- gi/pygi-argument.c | 626 --------------------------------------------------- gi/pygi-argument.h | 12 - gi/pygi-info.c | 90 ++++++-- tests/test_fields.py | 14 +- 4 files changed, 83 insertions(+), 659 deletions(-) diff --git a/gi/pygi-argument.c b/gi/pygi-argument.c index 2aaf4353..0c322e8c 100644 --- a/gi/pygi-argument.c +++ b/gi/pygi-argument.c @@ -131,632 +131,6 @@ _pygi_arg_to_hash_pointer (const GIArgument *arg, } } -static void -_pygi_g_type_tag_py_bounds (GITypeTag type_tag, - PyObject **lower, - PyObject **upper) -{ - switch (type_tag) { - case GI_TYPE_TAG_INT8: - *lower = PYGLIB_PyLong_FromLong (-128); - *upper = PYGLIB_PyLong_FromLong (127); - break; - case GI_TYPE_TAG_UINT8: - *upper = PYGLIB_PyLong_FromLong (255); - *lower = PYGLIB_PyLong_FromLong (0); - break; - case GI_TYPE_TAG_INT16: - *lower = PYGLIB_PyLong_FromLong (-32768); - *upper = PYGLIB_PyLong_FromLong (32767); - break; - case GI_TYPE_TAG_UINT16: - *upper = PYGLIB_PyLong_FromLong (65535); - *lower = PYGLIB_PyLong_FromLong (0); - break; - case GI_TYPE_TAG_INT32: - *lower = PYGLIB_PyLong_FromLong (G_MININT32); - *upper = PYGLIB_PyLong_FromLong (G_MAXINT32); - break; - case GI_TYPE_TAG_UINT32: - /* Note: On 32-bit archs, this number doesn't fit in a long. */ - *upper = PyLong_FromLongLong (G_MAXUINT32); - *lower = PYGLIB_PyLong_FromLong (0); - break; - case GI_TYPE_TAG_INT64: - /* Note: On 32-bit archs, these numbers don't fit in a long. */ - *lower = PyLong_FromLongLong (G_MININT64); - *upper = PyLong_FromLongLong (G_MAXINT64); - break; - case GI_TYPE_TAG_UINT64: - *upper = PyLong_FromUnsignedLongLong (G_MAXUINT64); - *lower = PYGLIB_PyLong_FromLong (0); - break; - case GI_TYPE_TAG_FLOAT: - *upper = PyFloat_FromDouble (G_MAXFLOAT); - *lower = PyFloat_FromDouble (-G_MAXFLOAT); - break; - case GI_TYPE_TAG_DOUBLE: - *upper = PyFloat_FromDouble (G_MAXDOUBLE); - *lower = PyFloat_FromDouble (-G_MAXDOUBLE); - break; - default: - PyErr_SetString (PyExc_TypeError, "Non-numeric type tag"); - *lower = *upper = NULL; - return; - } -} - -gint -_pygi_g_registered_type_info_check_object (GIRegisteredTypeInfo *info, - gboolean is_instance, - PyObject *object) -{ - gint retval; - - GType g_type; - PyObject *py_type; - gchar *type_name_expected = NULL; - GIInfoType interface_type; - - interface_type = g_base_info_get_type (info); - if ( (interface_type == GI_INFO_TYPE_STRUCT) && - (g_struct_info_is_foreign ( (GIStructInfo*) info))) { - /* TODO: Could we check is the correct foreign type? */ - return 1; - } - - g_type = g_registered_type_info_get_g_type (info); - if (g_type != G_TYPE_NONE) { - py_type = _pygi_type_get_from_g_type (g_type); - } else { - py_type = _pygi_type_import_by_gi_info ( (GIBaseInfo *) info); - } - - if (py_type == NULL) { - return 0; - } - - g_assert (PyType_Check (py_type)); - - if (is_instance) { - retval = PyObject_IsInstance (object, py_type); - if (!retval) { - type_name_expected = _pygi_g_base_info_get_fullname ( - (GIBaseInfo *) info); - } - } else { - if (!PyObject_Type (py_type)) { - type_name_expected = "type"; - retval = 0; - } else if (!PyType_IsSubtype ( (PyTypeObject *) object, - (PyTypeObject *) py_type)) { - type_name_expected = _pygi_g_base_info_get_fullname ( - (GIBaseInfo *) info); - retval = 0; - } else { - retval = 1; - } - } - - Py_DECREF (py_type); - - if (!retval) { - PyTypeObject *object_type; - - if (type_name_expected == NULL) { - return -1; - } - - object_type = (PyTypeObject *) PyObject_Type (object); - if (object_type == NULL) { - return -1; - } - - PyErr_Format (PyExc_TypeError, "Must be %s, not %s", - type_name_expected, object_type->tp_name); - - g_free (type_name_expected); - } - - return retval; -} - -gint -_pygi_g_type_interface_check_object (GIBaseInfo *info, - PyObject *object) -{ - gint retval = 1; - GIInfoType info_type; - - info_type = g_base_info_get_type (info); - switch (info_type) { - case GI_INFO_TYPE_CALLBACK: - if (!PyCallable_Check (object)) { - PyErr_Format (PyExc_TypeError, "Must be callable, not %s", - object->ob_type->tp_name); - retval = 0; - } - break; - case GI_INFO_TYPE_ENUM: - retval = 0; - if (PyNumber_Check (object)) { - PyObject *number = PYGLIB_PyNumber_Long (object); - if (number == NULL) - PyErr_Clear(); - else { - glong value = PYGLIB_PyLong_AsLong (number); - int i; - for (i = 0; i < g_enum_info_get_n_values (info); i++) { - GIValueInfo *value_info = g_enum_info_get_value (info, i); - glong enum_value = g_value_info_get_value (value_info); - g_base_info_unref (value_info); - if (value == enum_value) { - retval = 1; - break; - } - } - } - } - if (retval < 1) - retval = _pygi_g_registered_type_info_check_object ( - (GIRegisteredTypeInfo *) info, TRUE, object); - break; - case GI_INFO_TYPE_FLAGS: - if (PyNumber_Check (object)) { - /* Accept 0 as a valid flag value */ - PyObject *number = PYGLIB_PyNumber_Long (object); - if (number == NULL) - PyErr_Clear(); - else { - long value = PYGLIB_PyLong_AsLong (number); - if (value == 0) - break; - else if (value == -1) - PyErr_Clear(); - } - } - retval = _pygi_g_registered_type_info_check_object ( - (GIRegisteredTypeInfo *) info, TRUE, object); - break; - case GI_INFO_TYPE_STRUCT: - { - GType type; - - /* Handle special cases. */ - type = g_registered_type_info_get_g_type ( (GIRegisteredTypeInfo *) info); - if (g_type_is_a (type, G_TYPE_CLOSURE)) { - if (!(PyCallable_Check (object) || - pyg_type_from_object_strict (object, FALSE) == G_TYPE_CLOSURE)) { - PyErr_Format (PyExc_TypeError, "Must be callable, not %s", - object->ob_type->tp_name); - retval = 0; - } - break; - } else if (g_type_is_a (type, G_TYPE_VALUE)) { - /* we can't check g_values because we don't have - * enough context so just pass them through */ - break; - } - - /* Fallback. */ - } - case GI_INFO_TYPE_BOXED: - case GI_INFO_TYPE_INTERFACE: - case GI_INFO_TYPE_OBJECT: - retval = _pygi_g_registered_type_info_check_object ( (GIRegisteredTypeInfo *) info, TRUE, object); - break; - case GI_INFO_TYPE_UNION: - - - retval = _pygi_g_registered_type_info_check_object ( (GIRegisteredTypeInfo *) info, TRUE, object); - - /* If not the same type then check to see if the object's type - * is the same as one of the union's members - */ - if (retval == 0) { - gint i; - gint n_fields; - - n_fields = g_union_info_get_n_fields ( (GIUnionInfo *) info); - - for (i = 0; i < n_fields; i++) { - gint member_retval; - GIFieldInfo *field_info; - GITypeInfo *field_type_info; - - field_info = - g_union_info_get_field ( (GIUnionInfo *) info, i); - field_type_info = g_field_info_get_type (field_info); - - member_retval = _pygi_g_type_info_check_object( - field_type_info, - object, - TRUE); - - g_base_info_unref ( ( GIBaseInfo *) field_type_info); - g_base_info_unref ( ( GIBaseInfo *) field_info); - - if (member_retval == 1) { - retval = member_retval; - break; - } - } - } - - break; - default: - g_assert_not_reached(); - } - - return retval; -} - -gint -_pygi_g_type_info_check_object (GITypeInfo *type_info, - PyObject *object, - gboolean allow_none) -{ - GITypeTag type_tag; - gint retval = 1; - - if (allow_none && object == Py_None) { - return retval; - } - - type_tag = g_type_info_get_tag (type_info); - - switch (type_tag) { - case GI_TYPE_TAG_VOID: - /* No check; VOID means undefined type */ - break; - case GI_TYPE_TAG_BOOLEAN: - /* No check; every Python object has a truth value. */ - break; - case GI_TYPE_TAG_UINT8: - case GI_TYPE_TAG_INT8: - /* (U)INT8 types can be characters */ - if (PYGLIB_PyBytes_Check(object)) { - if (PYGLIB_PyBytes_Size(object) != 1) { - PyErr_Format (PyExc_TypeError, "Must be a single character"); - retval = 0; - break; - } - - break; - } - case GI_TYPE_TAG_INT16: - case GI_TYPE_TAG_UINT16: - case GI_TYPE_TAG_INT32: - case GI_TYPE_TAG_UINT32: - case GI_TYPE_TAG_INT64: - case GI_TYPE_TAG_UINT64: - case GI_TYPE_TAG_FLOAT: - case GI_TYPE_TAG_DOUBLE: - { - PyObject *number, *lower, *upper; - - if (!PyNumber_Check (object)) { - PyErr_Format (PyExc_TypeError, "Must be number, not %s", - object->ob_type->tp_name); - retval = 0; - break; - } - - if (type_tag == GI_TYPE_TAG_FLOAT || type_tag == GI_TYPE_TAG_DOUBLE) { - number = PyNumber_Float (object); - } else { - number = PYGLIB_PyNumber_Long (object); - } - - _pygi_g_type_tag_py_bounds (type_tag, &lower, &upper); - - if (lower == NULL || upper == NULL || number == NULL) { - retval = -1; - goto check_number_release; - } - - /* Check bounds */ - if (PyObject_RichCompareBool (lower, number, Py_GT) - || PyObject_RichCompareBool (upper, number, Py_LT)) { - PyObject *lower_str; - PyObject *upper_str; - - if (PyErr_Occurred()) { - retval = -1; - goto check_number_release; - } - - lower_str = PyObject_Str (lower); - upper_str = PyObject_Str (upper); - if (lower_str == NULL || upper_str == NULL) { - retval = -1; - goto check_number_error_release; - } - -#if PY_VERSION_HEX < 0x03000000 - PyErr_Format (PyExc_ValueError, "Must range from %s to %s", - PyString_AS_STRING (lower_str), - PyString_AS_STRING (upper_str)); -#else - { - PyObject *lower_pybytes_obj; - PyObject *upper_pybytes_obj; - - lower_pybytes_obj = PyUnicode_AsUTF8String (lower_str); - if (!lower_pybytes_obj) { - goto utf8_fail; - } - - upper_pybytes_obj = PyUnicode_AsUTF8String (upper_str); - if (!upper_pybytes_obj) { - Py_DECREF(lower_pybytes_obj); - goto utf8_fail; - } - - PyErr_Format (PyExc_ValueError, "Must range from %s to %s", - PyBytes_AsString (lower_pybytes_obj), - PyBytes_AsString (upper_pybytes_obj)); - Py_DECREF (lower_pybytes_obj); - Py_DECREF (upper_pybytes_obj); - } -utf8_fail: -#endif - retval = 0; - -check_number_error_release: - Py_XDECREF (lower_str); - Py_XDECREF (upper_str); - } - -check_number_release: - Py_XDECREF (number); - Py_XDECREF (lower); - Py_XDECREF (upper); - break; - } - case GI_TYPE_TAG_GTYPE: - { - if (pyg_type_from_object (object) == 0) { - PyErr_Format (PyExc_TypeError, "Must be gobject.GType, not %s", - object->ob_type->tp_name); - retval = 0; - } - break; - } - case GI_TYPE_TAG_UNICHAR: - { - Py_ssize_t size; - if (PyUnicode_Check (object)) { - size = PyUnicode_GET_SIZE (object); -#if PY_VERSION_HEX < 0x03000000 - } else if (PyString_Check (object)) { - PyObject *pyuni = PyUnicode_FromEncodedObject (object, "UTF-8", "strict"); - size = PyUnicode_GET_SIZE (pyuni); - Py_DECREF(pyuni); -#endif - } else { - PyErr_Format (PyExc_TypeError, "Must be string, not %s", - object->ob_type->tp_name); - retval = 0; - break; - } - - if (size != 1) { - PyErr_Format (PyExc_TypeError, "Must be a one character string, not %" G_GINT64_FORMAT " characters", - (gint64)size); - retval = 0; - break; - } - - break; - } - case GI_TYPE_TAG_UTF8: - case GI_TYPE_TAG_FILENAME: - if (!PYGLIB_PyBaseString_Check (object) ) { - PyErr_Format (PyExc_TypeError, "Must be string, not %s", - object->ob_type->tp_name); - retval = 0; - } - break; - case GI_TYPE_TAG_ARRAY: - { - gssize fixed_size; - Py_ssize_t length; - GITypeInfo *item_type_info; - Py_ssize_t i; - - if (!PySequence_Check (object)) { - PyErr_Format (PyExc_TypeError, "Must be sequence, not %s", - object->ob_type->tp_name); - retval = 0; - break; - } - - length = PySequence_Length (object); - if (length < 0) { - retval = -1; - break; - } - - fixed_size = g_type_info_get_array_fixed_size (type_info); - if (fixed_size >= 0 && length != fixed_size) { - PyErr_Format (PyExc_ValueError, "Must contain %zd items, not %zd", - fixed_size, length); - retval = 0; - break; - } - - item_type_info = g_type_info_get_param_type (type_info, 0); - g_assert (item_type_info != NULL); - - /* FIXME: This is insain. We really should only check the first - * object and perhaps have a debugging mode. Large arrays - * will cause apps to slow to a crawl. - */ - for (i = 0; i < length; i++) { - PyObject *item; - - item = PySequence_GetItem (object, i); - if (item == NULL) { - retval = -1; - break; - } - - retval = _pygi_g_type_info_check_object (item_type_info, item, TRUE); - - Py_DECREF (item); - - if (retval < 0) { - break; - } - if (!retval) { - _PyGI_ERROR_PREFIX ("Item %zd: ", i); - break; - } - } - - g_base_info_unref ( (GIBaseInfo *) item_type_info); - - break; - } - case GI_TYPE_TAG_INTERFACE: - { - GIBaseInfo *info; - - info = g_type_info_get_interface (type_info); - g_assert (info != NULL); - - retval = _pygi_g_type_interface_check_object(info, object); - - g_base_info_unref (info); - break; - } - case GI_TYPE_TAG_GLIST: - case GI_TYPE_TAG_GSLIST: - { - Py_ssize_t length; - GITypeInfo *item_type_info; - Py_ssize_t i; - - if (!PySequence_Check (object)) { - PyErr_Format (PyExc_TypeError, "Must be sequence, not %s", - object->ob_type->tp_name); - retval = 0; - break; - } - - length = PySequence_Length (object); - if (length < 0) { - retval = -1; - break; - } - - item_type_info = g_type_info_get_param_type (type_info, 0); - g_assert (item_type_info != NULL); - - for (i = 0; i < length; i++) { - PyObject *item; - - item = PySequence_GetItem (object, i); - if (item == NULL) { - retval = -1; - break; - } - - retval = _pygi_g_type_info_check_object (item_type_info, item, TRUE); - - Py_DECREF (item); - - if (retval < 0) { - break; - } - if (!retval) { - _PyGI_ERROR_PREFIX ("Item %zd: ", i); - break; - } - } - - g_base_info_unref ( (GIBaseInfo *) item_type_info); - break; - } - case GI_TYPE_TAG_GHASH: - { - Py_ssize_t length; - PyObject *keys; - PyObject *values; - GITypeInfo *key_type_info; - GITypeInfo *value_type_info; - Py_ssize_t i; - - keys = PyMapping_Keys (object); - if (keys == NULL) { - PyErr_Format (PyExc_TypeError, "Must be mapping, not %s", - object->ob_type->tp_name); - retval = 0; - break; - } - - length = PyMapping_Length (object); - if (length < 0) { - Py_DECREF (keys); - retval = -1; - break; - } - - values = PyMapping_Values (object); - if (values == NULL) { - retval = -1; - Py_DECREF (keys); - break; - } - - key_type_info = g_type_info_get_param_type (type_info, 0); - g_assert (key_type_info != NULL); - - value_type_info = g_type_info_get_param_type (type_info, 1); - g_assert (value_type_info != NULL); - - for (i = 0; i < length; i++) { - PyObject *key; - PyObject *value; - - key = PyList_GET_ITEM (keys, i); - value = PyList_GET_ITEM (values, i); - - retval = _pygi_g_type_info_check_object (key_type_info, key, TRUE); - if (retval < 0) { - break; - } - if (!retval) { - _PyGI_ERROR_PREFIX ("Key %zd :", i); - break; - } - - retval = _pygi_g_type_info_check_object (value_type_info, value, TRUE); - if (retval < 0) { - break; - } - if (!retval) { - _PyGI_ERROR_PREFIX ("Value %zd :", i); - break; - } - } - - g_base_info_unref ( (GIBaseInfo *) key_type_info); - g_base_info_unref ( (GIBaseInfo *) value_type_info); - Py_DECREF (values); - Py_DECREF (keys); - break; - } - case GI_TYPE_TAG_ERROR: - PyErr_SetString (PyExc_NotImplementedError, "Error marshalling is not supported yet"); - /* TODO */ - break; - } - - return retval; -} - /** * _pygi_argument_array_length_marshal: diff --git a/gi/pygi-argument.h b/gi/pygi-argument.h index 97ff393f..a923fd94 100644 --- a/gi/pygi-argument.h +++ b/gi/pygi-argument.h @@ -42,18 +42,6 @@ gpointer _pygi_arg_to_hash_pointer (const GIArgument *arg, void _pygi_hash_pointer_to_arg (GIArgument *arg, GITypeTag type_tag); -gint _pygi_g_type_interface_check_object (GIBaseInfo *info, - PyObject *object); - -gint _pygi_g_type_info_check_object (GITypeInfo *type_info, - PyObject *object, - gboolean allow_none); - -gint _pygi_g_registered_type_info_check_object (GIRegisteredTypeInfo *info, - gboolean is_instance, - PyObject *object); - - GArray* _pygi_argument_to_array (GIArgument *arg, PyGIArgArrayLengthPolicy array_length_policy, void *user_data1, diff --git a/gi/pygi-info.c b/gi/pygi-info.c index 8e0892a2..cc8656b4 100644 --- a/gi/pygi-info.c +++ b/gi/pygi-info.c @@ -1812,6 +1812,81 @@ out: return array_len; } +static gint +_pygi_g_registered_type_info_check_object (GIRegisteredTypeInfo *info, + gboolean is_instance, + PyObject *object) +{ + gint retval; + + GType g_type; + PyObject *py_type; + gchar *type_name_expected = NULL; + GIInfoType interface_type; + + interface_type = g_base_info_get_type (info); + if ( (interface_type == GI_INFO_TYPE_STRUCT) && + (g_struct_info_is_foreign ( (GIStructInfo*) info))) { + /* TODO: Could we check is the correct foreign type? */ + return 1; + } + + g_type = g_registered_type_info_get_g_type (info); + if (g_type != G_TYPE_NONE) { + py_type = _pygi_type_get_from_g_type (g_type); + } else { + py_type = _pygi_type_import_by_gi_info ( (GIBaseInfo *) info); + } + + if (py_type == NULL) { + return 0; + } + + g_assert (PyType_Check (py_type)); + + if (is_instance) { + retval = PyObject_IsInstance (object, py_type); + if (!retval) { + type_name_expected = _pygi_g_base_info_get_fullname ( + (GIBaseInfo *) info); + } + } else { + if (!PyObject_Type (py_type)) { + type_name_expected = "type"; + retval = 0; + } else if (!PyType_IsSubtype ( (PyTypeObject *) object, + (PyTypeObject *) py_type)) { + type_name_expected = _pygi_g_base_info_get_fullname ( + (GIBaseInfo *) info); + retval = 0; + } else { + retval = 1; + } + } + + Py_DECREF (py_type); + + if (!retval) { + PyTypeObject *object_type; + + if (type_name_expected == NULL) { + return -1; + } + + object_type = (PyTypeObject *) PyObject_Type (object); + if (object_type == NULL) { + return -1; + } + + PyErr_Format (PyExc_TypeError, "Must be %s, not %s", + type_name_expected, object_type->tp_name); + + g_free (type_name_expected); + } + + return retval; +} + static PyObject * _wrap_g_field_info_get_value (PyGIBaseInfo *self, PyObject *args) @@ -1965,21 +2040,6 @@ _wrap_g_field_info_set_value (PyGIBaseInfo *self, field_type_info = g_field_info_get_type ( (GIFieldInfo *) self->info); - /* Check the value. */ - { - gboolean retval; - - retval = _pygi_g_type_info_check_object (field_type_info, py_value, TRUE); - if (retval < 0) { - goto out; - } - - if (!retval) { - _PyGI_ERROR_PREFIX ("argument 2: "); - goto out; - } - } - /* Set the field's value. */ /* A few types are not handled by g_field_info_set_field, so do it here. */ if (!g_type_info_is_pointer (field_type_info) diff --git a/tests/test_fields.py b/tests/test_fields.py index e3403ca5..def3511c 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -35,8 +35,8 @@ class TestFields(unittest.TestCase): self.assertRaises(TypeError, setattr, s, "some_int8", b"ab") self.assertRaises(TypeError, setattr, s, "some_int8", None) - self.assertRaises(ValueError, setattr, s, "some_int8", 128) - self.assertRaises(ValueError, setattr, s, "some_int8", -129) + self.assertRaises(OverflowError, setattr, s, "some_int8", 128) + self.assertRaises(OverflowError, setattr, s, "some_int8", -129) s.some_int8 = 3.6 self.assertEqual(s.some_int8, 3) @@ -51,8 +51,10 @@ class TestFields(unittest.TestCase): self.assertRaises(TypeError, setattr, s, "some_int", b"a") self.assertRaises(TypeError, setattr, s, "some_int", None) - self.assertRaises(ValueError, setattr, s, "some_int", GLib.MAXINT + 1) - self.assertRaises(ValueError, setattr, s, "some_int", GLib.MININT - 1) + self.assertRaises( + OverflowError, setattr, s, "some_int", GLib.MAXINT + 1) + self.assertRaises( + OverflowError, setattr, s, "some_int", GLib.MININT - 1) s.some_int = 3.6 self.assertEqual(s.some_int, 3) @@ -67,8 +69,8 @@ class TestFields(unittest.TestCase): self.assertRaises(TypeError, setattr, s, "long_", b"a") self.assertRaises(TypeError, setattr, s, "long_", None) - self.assertRaises(ValueError, setattr, s, "long_", GLib.MAXLONG + 1) - self.assertRaises(ValueError, setattr, s, "long_", GLib.MINLONG - 1) + self.assertRaises(OverflowError, setattr, s, "long_", GLib.MAXLONG + 1) + self.assertRaises(OverflowError, setattr, s, "long_", GLib.MINLONG - 1) s.long_ = 3.6 self.assertEqual(s.long_, 3) -- 2.11.4.GIT