This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB 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: patch for #14363


On 06.08.2012 12:42, Phil Muldoon wrote:
On 08/02/2012 05:21 PM, Oliver Buchtala wrote:
 From 05a921d423793db93dfead59c9fe12d342f55d75 Mon Sep 17 00:00:00 2001
From: Oliver Buchtala <oliver.buchtala@googlemail.com>
Date: Wed, 1 Aug 2012 21:59:43 +0200
Subject: [PATCH] Adapt py-prettyprint to inform pretty-printer about current
  recursion level.

---
  gdb/python/py-prettyprint.c  |    3 +++
  gdb/python/python-internal.h |    1 +
  gdb/python/python.c          |    2 ++
  3 files changed, 6 insertions(+)
Thanks.

Needs a GNU style ChangeLog, testcase and documentation
additions.

I am having trouble understanding what you are doing with "recurse"
attribute.  From valprint.c, in the GDB sources (which calls
apply_val_pretty_printer, among others):

    "RECURSE indicates the amount of indentation to supply before
    continuation lines; this amount is roughly twice the value of
    RECURSE."

So I am not sure that this is the value you want?  Anyway, I think you
want the number of recursions for that printer, not the recurse value
being supplied to apply_val_pretty_printer.  A testcase would help
here to understand your intention.

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 86d4f2c..d5c6b66 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -476,6 +476,9 @@ print_children (PyObject *printer, const char *hint,
if (! PyObject_HasAttr (printer, gdbpy_children_cst))
return;
+ if ( PyObject_HasAttr (printer, gdbpy_level_cst))
+ PyObject_SetAttr(printer, gdbpy_level_cst, PyInt_FromLong((long) recurse));
+
There are a few issues with this patch hunk:

* It is not clear in the documentation, but PyInt_FromLong can raise a
   PyErr_NoMemory exception, so you need to NULL check the return and
   dispatch the exception appropriately.

* This leaks a reference from PyInt_FromLong.  PyObject_SetAttr
   increments the reference count of the right side of "=" Python
   equivalent (IE foo.bar = baz), which in this case is the integer. So
   the transient reference from PyInt_FromLong will last forever.  You
   you need to assign the results of PyInt_FromLong to a variable and
   call Py_DECREF on it after the "PyObject_SetAttr" call.

* PyObject_SetAttr can fail, so the code needs to deal with this
   contingency.

* Why PyInt_FromLong over PyLong_FromLong?

Beyond the patch fixes, is it not possible to count the recursion
level by some internal bookkeeping in the printer?  I have no problem
with including the attribute if it is helpful, or clearer to solve the
problem, but I am curious if you have tried the alternative, if
possible?

Cheers,

Phil
Thanks,

I see there are lots of deficiencies with my patch. Sorry for that.
You are right in all your points.
I was not aware that recurse == indentation. (misnaming? ;) )

For the second proposal, I need a set data structure for longs.
Do you have such a data structure in gdb somewhere? Or which should I use?
Then I would offer a prototype implementation.

Regards,
Oliver


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