This is the mail archive of the archer@sourceware.org mailing list for the Archer project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [python][rfc] convert_value_from_python changes inpretty-printing


El jue, 05-02-2009 a las 13:17 -0700, Tom Tromey escribiÃ:
> >>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:
> 
> Thiago> Most callers were easy to adapt, but I'm not sure about the
> Thiago> changes I made in pretty_print_one_value and print_children,
> Thiago> could you take a look?
> 
> It looks like there is a lurking bug in pretty_print_one_value -- it
> calls gdbpy_print_stack, but so does one of its callers,
> print_string_repr.
> 
> I suggest removing the call from pretty_print_one_value and putting it
> into its callers.  WDYT?

Sounds good. I committed the following then. This patch also has the
part which went in upstream.

> Thiago> +	      error(_("Error while executing Python code."));
> 
> The space before the "(" is your nemesis :-)

I guess I didn't drink enough of the GNU Coding Standards kool-aid. :-)
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


2009-02-08  Thiago Jung Bauermann  <bauerman@br.ibm.com>

	* python/python-value.c (convert_value_from_python): Change to throw
	a Python exception instead of a GDB exception on error.
	(valpy_new, valpy_getitem, valpy_binop, valpy_richcompare): Adapt to
	new behaviour of convert_value_from_python.
	* python/python-function.c (fnpy_call): Likewise.
	* python/python.c (pretty_print_one_value, print_children): Likewise.
	(apply_varobj_pretty_printer): Call gdbpy_print_stack if
	pretty_print_one_value fails.

diff --git a/gdb/python/python-function.c b/gdb/python/python-function.c
index 840c17e..5e346d6 100644
--- a/gdb/python/python-function.c
+++ b/gdb/python/python-function.c
@@ -58,7 +58,6 @@ fnpy_call (void *cookie, int argc, struct value **argv)
   int i;
   struct value *value = NULL;
   PyObject *result, *callable, *args;
-  volatile struct gdb_exception except;
   struct cleanup *cleanup;
   PyGILState_STATE state;
 
@@ -81,18 +80,15 @@ fnpy_call (void *cookie, int argc, struct value **argv)
   if (!result)
     {
       gdbpy_print_stack ();
-      error(_("Error while executing Python code."));
+      error (_("Error while executing Python code."));
     }
 
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      value = convert_value_from_python (result);
-    }
-  if (except.reason < 0)
+  value = convert_value_from_python (result);
+  if (value == NULL)
     {
       Py_DECREF (result);
       gdbpy_print_stack ();
-      throw_exception (except);
+      error (_("Error while executing Python code."));
     }
 
   Py_DECREF (result);
diff --git a/gdb/python/python-value.c b/gdb/python/python-value.c
index d29bc56..8b817a1 100644
--- a/gdb/python/python-value.c
+++ b/gdb/python/python-value.c
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "gdb_assert.h"
 #include "charset.h"
 #include "value.h"
 #include "exceptions.h"
@@ -79,7 +80,6 @@ valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
 {
   struct value *value = NULL;   /* Initialize to appease gcc warning.  */
   value_object *value_obj;
-  volatile struct gdb_exception except;
 
   if (PyTuple_Size (args) != 1)
     {
@@ -96,16 +96,11 @@ valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
       return NULL;
     }
 
-  TRY_CATCH (except, RETURN_MASK_ALL)
-    {
-      value = convert_value_from_python (PyTuple_GetItem (args, 0));
-    }
-  if (except.reason < 0)
+  value = convert_value_from_python (PyTuple_GetItem (args, 0));
+  if (value == NULL)
     {
       subtype->tp_free (value_obj);
-      return PyErr_Format (except.reason == RETURN_QUIT
-			     ? PyExc_KeyboardInterrupt : PyExc_TypeError,
-			     "%s", except.message);
+      return NULL;
     }
 
   value_obj->value = value;
@@ -254,6 +249,9 @@ valpy_getitem (PyObject *self, PyObject *key)
 	     value code throw an exception if the index has an invalid
 	     type.  */
 	  struct value *idx = convert_value_from_python (key);
+	  if (idx == NULL)
+	    return NULL;
+
 	  res_val = value_subscript (tmp, idx);
 	}
     }
@@ -343,7 +341,12 @@ valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 	 kind of object, altogether.  Because of this, we can't assume self is
 	 a gdb.Value object and need to convert it from python as well.  */
       arg1 = convert_value_from_python (self);
+      if (arg1 == NULL)
+	return NULL;
+
       arg2 = convert_value_from_python (other);
+      if (arg2 == NULL)
+	return NULL;
 
       switch (opcode)
 	{
@@ -607,6 +610,8 @@ valpy_richcompare (PyObject *self, PyObject *other, int op)
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
       value_other = convert_value_from_python (other);
+      if (value_other == NULL)
+	return NULL;
 
       switch (op) {
         case Py_LT:
@@ -761,7 +766,7 @@ value_object_to_value (PyObject *self)
 }
 
 /* Try to convert a Python value to a gdb value.  If the value cannot
-   be converted, throw a gdb exception.  */
+   be converted, set a Python exception and return NULL.  */
 
 struct value *
 convert_value_from_python (PyObject *obj)
@@ -769,54 +774,65 @@ convert_value_from_python (PyObject *obj)
   struct value *value = NULL; /* -Wall */
   PyObject *target_str, *unicode_str;
   struct cleanup *old;
+  volatile struct gdb_exception except;
   int cmp;
 
-  if (! obj)
-    error (_("Internal error while converting Python value."));
+  gdb_assert (obj != NULL);
 
-  if (PyBool_Check (obj)) 
-    {
-      cmp = PyObject_IsTrue (obj);
-      if (cmp >= 0)
-	value = value_from_longest (builtin_type_pybool, cmp);
-    }
-  else if (PyInt_Check (obj))
-    value = value_from_longest (builtin_type_pyint, PyInt_AsLong (obj));
-  else if (PyLong_Check (obj))
-    {
-      LONGEST l = PyLong_AsLongLong (obj);
-      if (! PyErr_Occurred ())
-	value = value_from_longest (builtin_type_pylong, l);
-    }
-  else if (PyFloat_Check (obj))
-    {
-      double d = PyFloat_AsDouble (obj);
-      if (! PyErr_Occurred ())
-	value = value_from_double (builtin_type_pyfloat, d);
-    }
-  else if (gdbpy_is_string (obj))
+  TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      char *s;
+      if (PyBool_Check (obj)) 
+	{
+	  cmp = PyObject_IsTrue (obj);
+	  if (cmp >= 0)
+	    value = value_from_longest (builtin_type_pybool, cmp);
+	}
+      else if (PyInt_Check (obj))
+	{
+	  long l = PyInt_AsLong (obj);
 
-      s = python_string_to_target_string (obj);
-      if (s == NULL)
+	  if (! PyErr_Occurred ())
+	    value = value_from_longest (builtin_type_pyint, l);
+	}
+      else if (PyLong_Check (obj))
 	{
-	  PyErr_Clear ();
-	  error (_("Error converting Python value."));
+	  LONGEST l = PyLong_AsLongLong (obj);
+
+	  if (! PyErr_Occurred ())
+	    value = value_from_longest (builtin_type_pylong, l);
 	}
+      else if (PyFloat_Check (obj))
+	{
+	  double d = PyFloat_AsDouble (obj);
 
-      old = make_cleanup (xfree, s);
-      value = value_from_string (s);
-      do_cleanups (old);
+	  if (! PyErr_Occurred ())
+	    value = value_from_double (builtin_type_pyfloat, d);
+	}
+      else if (gdbpy_is_string (obj))
+	{
+	  char *s;
+
+	  s = python_string_to_target_string (obj);
+	  if (s != NULL)
+	    {
+	      old = make_cleanup (xfree, s);
+	      value = value_from_string (s);
+	      do_cleanups (old);
+	    }
+	}
+      else if (PyObject_TypeCheck (obj, &value_object_type))
+	value = ((value_object *) obj)->value;
+      else
+	PyErr_Format (PyExc_TypeError, _("Could not convert Python object: %s"),
+		      PyString_AsString (PyObject_Str (obj)));
+    }
+  if (except.reason < 0)
+    {
+      PyErr_Format (except.reason == RETURN_QUIT
+		    ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
+		    "%s", except.message);
+      return NULL;
     }
-  else if (PyObject_TypeCheck (obj, &value_object_type))
-    value = ((value_object *) obj)->value;
-  else
-    error (_("Could not convert Python object: %s"),
-	   PyString_AsString (PyObject_Str (obj)));
-
-  if (PyErr_Occurred ())
-    error (_("Error converting Python value."));
 
   return value;
 }
diff --git a/gdb/python/python.c b/gdb/python/python.c
index a3a8bdf..8677da6 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -958,8 +958,6 @@ pretty_print_one_value (PyObject *printer, struct value **out_value)
 	    *out_value = convert_value_from_python (result);
 	  Py_DECREF (result);
 	}
-      else
-	gdbpy_print_stack ();
     }
 
   return output;
@@ -1252,7 +1250,14 @@ print_children (PyObject *printer, const char *hint,
       else
 	{
 	  struct value *value = convert_value_from_python (py_v);
-	  common_val_print (value, stream, recurse + 1, options, language);
+
+	  if (value == NULL)
+	    {
+	      gdbpy_print_stack ();
+	      error (_("Error while executing Python code."));
+	    }
+	  else
+	    common_val_print (value, stream, recurse + 1, options, language);
 	}
 
       if (is_map && i % 2 == 0)
@@ -1339,7 +1344,7 @@ apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
 
 /* Apply a pretty-printer for the varobj code.  PRINTER_OBJ is the
    print object.  It must have a 'to_string' method (but this is
-   checked by varobj, not here) which accepts takes no arguments and
+   checked by varobj, not here) which takes no arguments and
    returns a string.  This function returns an xmalloc()d string if
    the printer returns a string.  The printer may return a replacement
    value instead; in this case *REPLACEMENT is set to the replacement
@@ -1354,6 +1359,8 @@ apply_varobj_pretty_printer (PyObject *printer_obj, struct value *value,
 
   *replacement = NULL;
   result = pretty_print_one_value (printer_obj, replacement);
+  if (result == NULL);
+    gdbpy_print_stack ();
   PyGILState_Release (state);
 
   return result;



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]