Re: Python pretty-printing [2/6]

[Thiago Jung Bauermann <bauerman@br.ibm.com>]

El jue, 02-04-2009 a las 14:55 -0600, Tom Tromey escribió:
> This patch adds a minimal Python wrapper for struct objfile,
> and arranges to auto-load Python code when an objfile is created.

Hooray. Just a few comments...

> +@findex gdb.current_objfile
> +@defun current_objfile
> +When auto-loading a Python script (@pxref{Auto-loading}), @value{GDBN}
> +sets the ``current objfile'' to the corresponding objfile.  This
> +function returns the current objfile.  If there is no current objfile,
> +this function returns @code{None}.
> +@end defun
> +
> +@findex gdb.objfiles
> +@defun objfiles
> +Return a sequence of all the objfiles current known to @value{GDBN}.
> +@xref{Objfiles In Python}.
> +@end defun

IMHO these should go in the "Objfiles In Python" section, like e.g.
gdb.selected_frame goes in the "Frames In Python" section.

> +static int
> +objfpy_set_printers (PyObject *o, PyObject *value, void *ignore)
<snip>
> +  Py_XDECREF (self->printers);
> +  Py_INCREF (value);
> +  self->printers = value;

I'm trying to decide if the sequence above is ok or not. Python docs
say[1] in a warning box:

"... any object that is reachable from a global variable should be in a
consistent state before Py_DECREF is invoked. For example, code to
delete an object from a list should copy a reference to the deleted
object in a temporary variable, update the list data structure, and then
call Py_DECREF for the temporary variable."

So perhaps you should:

  tmp = self->printers;
  Py_INCREF (value);
  self->printers = value;
  Py_XDECREF (tmp);

You certainly have more experience in this area than me though. WDYT?

> +/* Return the Python object of type Objfile representing OBJFILE.  If
> +   the object has already been created, return it.  Otherwise, create
> +   it.  Return NULL and set the Python error on failure.  */
> +PyObject *
> +objfile_to_objfile_object (struct objfile *objfile)

Perhaps it would be useful to mention that this function returns a
borrowed reference to the object?

Also, just to check: the lack of testcases is because you believe this
code is tested enough with python-prettyprint.exp in a later patch?

[1] - http://docs.python.org/c-api/refcounting.html#Py_DECREF
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center