Re: Python pretty-printing [5/6]

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

El mié, 08-04-2009 a las 18:52 -0600, Tom Tromey escribió:
> >>>>> "Thiago" == Thiago Jung Bauermann <bauerman@br.ibm.com> writes:
> Tom> +find_pretty_printer (PyObject *value)
> 
> Thiago> Also, if this function returns NULL, sometimes a Python exception will
> Thiago> be set, sometimes it won't. Is this intended? If so, this should be
> Thiago> noted.
> 
> I looked again and I don't see how it can return NULL without setting
> the exception.  Can you point it out to me?

I can't find it either, now. Should have pointed out the first time
around. :-( I probably thought search_pp_list could return NULL without
setting a Python exception, and because of this find_pretty_printer
would blindly return NULL too. But I can't find that case anymore. I
probably made a mistake.

I found one potential problem, which could cause the function to return
NULL without an exception being set (it's not the case I thought of
before, I think): suppose there's no objfile Python object when this
function is called, to the ALL_OBJFILES loop will skip all objs, then
the gdb module has no pretty_printers attribute, or the pretty_printers
value is not a list object. In that case, the function will return NULL
without a Python exception being set. Can it happen?

Also, I noticed that the function may return Py_None if no
pretty-printer is found, and the callers even expect that. The function
comment needs to be fixed then:

/* Find the pretty-printing constructor function for TYPE.  If no
   pretty-printer exists, return NULL.  If one exists, return a new
   reference.  */

> Thiago> Because of this, pretty_print_one_value can now probably just call
> Thiago> convert_value_from_python and return a struct value in all cases and be
> Thiago> done with it. Either this function should be changed to work that way,
> Thiago> or the comment above removed.
> 
> I made the minimal change here.  I don't want to refactor this right
> now; Phil is in the middle of doing that for the embedded \0 problem.
> We can apply his fix separately; the current code is not ideal, due to
> this problem, but it is still usable for a variety of printers.

Fine with me.

> Tom> +char *
> Tom> +gdbpy_get_display_hint (PyObject *printer)
> 
> Thiago> I use the *py_ prefix for functions that can be directly called from
> Thiago> Python. I think it's a useful hint. I don't think I ever mentioned this
> Thiago> practice though.
> 
> Thiago> If you agree it's useful, this method should be renamed to not use the
> Thiago> gdbpy_ prefix.
> 
> I am leaving this for now.  We use the gdbpy_ prefix inconsistently,
> even in CVS gdb, and I would prefer to see a single cleanup if we are
> going to adopt a solid naming convention.

Alright.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center