[RFA] Python: raise exception on field-related gdb.Type methods if it's not struct or union

[Paul Koning <paulkoning@comcast.net>]

On Nov 5, 2011, at 5:04 PM, Doug Evans wrote:

> On Sat, Nov 5, 2011 at 7:36 AM, Paul Koning <paulkoning@comcast.net> wrote:
>>> Maybe I'd be happy if gdb.Type (and maybe gdb.Value) were simply more
>>> rigorous in throwing exceptions for invalid cases.
>> 
>> I agree.  I'll put that together.
> 
> Sounds great if it's possible.
> 
> I don't know enough about the C Python API, but when I tried a simple
> hack to have len(scalar_type) throw an exception "not scalar_type"
> started throwing exceptions too.  :-(  See PyObject_IsTrue.

I found the answer to that question with a bit of searching.   Attached is a patch that raises an exception (TypeError) for len() and all the field reference methods if the type isn't struct or union.

Does this need new testcases?

	paul

ChangeLog:

2011-11-08  Paul Koning  <paul_koning@dell.com>

	* python/py-type.c (typy_deref): New function.
	(typy_nonzero): New function.
	(typy_length): Raise exception if not struct or union type.
	(typy_getitem): Ditto.
	(typy_has_key): Ditto.
	(typy_make_iter): Ditto.

Index: python/py-type.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-type.c,v
retrieving revision 1.28
diff -u -p -r1.28 py-type.c
--- python/py-type.c	4 Nov 2011 11:57:04 -0000	1.28
+++ python/py-type.c	8 Nov 2011 21:36:40 -0000
@@ -350,6 +350,45 @@ typy_strip_typedefs (PyObject *self, PyO
   return type_to_type_object (check_typedef (type));
 }
 
+/* Strip typedefs and pointers/reference from a type.  Then check that
+   it is a struct or union type.  If not, raise TypeError.  */
+static struct type *
+typy_deref (struct type *type)
+{
+  volatile struct gdb_exception except;
+
+  for (;;)
+    {
+      TRY_CATCH (except, RETURN_MASK_ALL)
+		{
+	  CHECK_TYPEDEF (type);
+	}
+      /* Don't use GDB_PY_HANDLE_EXCEPTION here because that returns
+	 a (NULL) pointer of the wrong type.  */
+      if (except.reason < 0)
+	{
+	  gdbpy_convert_exception (except);
+	  return NULL;
+	}
+
+      if (TYPE_CODE (type) != TYPE_CODE_PTR
+	  && TYPE_CODE (type) != TYPE_CODE_REF)
+	break;
+      type = TYPE_TARGET_TYPE (type);
+    }
+
+  /* If this is not a struct or union type, raise TypeError exception.  */
+  if (TYPE_CODE (type) != TYPE_CODE_STRUCT 
+      && TYPE_CODE (type) != TYPE_CODE_UNION)
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       "Type is not a structure or union type.");
+      return NULL;
+    }
+  
+  return type;
+}
+
 /* Return an array type.  */
 
 static PyObject *
@@ -1124,9 +1163,22 @@ typy_length (PyObject *self)
 {
   struct type *type = ((type_object *) self)->type;
 
+  type = typy_deref (type);
+  if (type == NULL)
+    return -1;
+
   return TYPE_NFIELDS (type);
 }
 
+/* Implements boolean evaluation of gdb.Type.  Handle this like other
+   Python objects that don't have a meaningful truth value -- all 
+   values are true.  */
+static int
+typy_nonzero (PyObject *self)
+{
+  return 1;
+}
+
 /* Return a gdb.Field object for the field named by the argument.  */
 
 static PyObject *
@@ -1145,20 +1197,10 @@ typy_getitem (PyObject *self, PyObject *
      using lookup_struct_elt_type, portions of that function are
      copied here.  */
 
-  for (;;)
-    {
-      TRY_CATCH (except, RETURN_MASK_ALL)
-	{
-	  CHECK_TYPEDEF (type);
-	}
-      GDB_PY_HANDLE_EXCEPTION (except);
-
-      if (TYPE_CODE (type) != TYPE_CODE_PTR
-	  && TYPE_CODE (type) != TYPE_CODE_REF)
-	break;
-      type = TYPE_TARGET_TYPE (type);
-    }
-
+  type = typy_deref (type);
+  if (type == NULL)
+    return NULL;
+  
   for (i = 0; i < TYPE_NFIELDS (type); i++)
     {
       char *t_field_name = TYPE_FIELD_NAME (type, i);
@@ -1216,18 +1258,9 @@ typy_has_key (PyObject *self, PyObject *
      using lookup_struct_elt_type, portions of that function are
      copied here.  */
 
-  for (;;)
-    {
-      TRY_CATCH (except, RETURN_MASK_ALL)
-	{
-	  CHECK_TYPEDEF (type);
-	}
-      GDB_PY_HANDLE_EXCEPTION (except);
-      if (TYPE_CODE (type) != TYPE_CODE_PTR
-	  && TYPE_CODE (type) != TYPE_CODE_REF)
-	break;
-      type = TYPE_TARGET_TYPE (type);
-    }
+  type = typy_deref (type);
+  if (type == NULL)
+    return NULL;
 
   for (i = 0; i < TYPE_NFIELDS (type); i++)
     {
@@ -1246,6 +1279,10 @@ typy_make_iter (PyObject *self, enum gdb
 {
   typy_iterator_object *typy_iter_obj;
 
+  /* Check that "self" is a structure or union type.  */
+  if (typy_deref (((type_object *) self)->type) == NULL)
+    return NULL;
+  
   typy_iter_obj = PyObject_New (typy_iterator_object,
 				&type_iterator_object_type);
   if (typy_iter_obj == NULL)
@@ -1499,6 +1536,32 @@ Return a volatile variant of this type" 
   { NULL }
 };
 
+static PyNumberMethods type_object_as_number = {
+  NULL,			      /* nb_add */
+  NULL,			      /* nb_subtract */
+  NULL,			      /* nb_multiply */
+  NULL,			      /* nb_divide */
+  NULL,			      /* nb_remainder */
+  NULL,			      /* nb_divmod */
+  NULL,			      /* nb_power */
+  NULL,			      /* nb_negative */
+  NULL,			      /* nb_positive */
+  NULL,			      /* nb_absolute */
+  typy_nonzero,		      /* nb_nonzero */
+  NULL,			      /* nb_invert */
+  NULL,			      /* nb_lshift */
+  NULL,			      /* nb_rshift */
+  NULL,			      /* nb_and */
+  NULL,			      /* nb_xor */
+  NULL,			      /* nb_or */
+  NULL,			      /* nb_coerce */
+  NULL,			      /* nb_int */
+  NULL,			      /* nb_long */
+  NULL,			      /* nb_float */
+  NULL,			      /* nb_oct */
+  NULL			      /* nb_hex */
+};
+
 static PyMappingMethods typy_mapping = {
   typy_length,
   typy_getitem,
@@ -1518,7 +1581,7 @@ static PyTypeObject type_object_type =
   0,				  /*tp_setattr*/
   0,				  /*tp_compare*/
   0,				  /*tp_repr*/
-  0,				  /*tp_as_number*/
+  &type_object_as_number,	  /*tp_as_number*/
   0,				  /*tp_as_sequence*/
   &typy_mapping,		  /*tp_as_mapping*/
   0,				  /*tp_hash */