[v2] Python API: Fix an exception when registering a global pretty-printer in verbose mode

Message ID 1426097817-30955-1-git-send-email-martin.galvan@tallertechnologies.com
State New, archived
Headers

Commit Message

Martin Galvan March 11, 2015, 6:16 p.m. UTC
  This patch fixes a Python exception that was being thrown when trying to register a global pretty-printer with verbose mode on:

  File "/usr/share/gdb/python/gdb/printing.py", line 119, in register_pretty_printer
    gdb.write("Registering global %s pretty-printer ...\n" % name)
NameError: name 'name' is not defined

My copyright assignment is on the works, but since this is a small patch I don't think it's necessary.

Changes from v1:
 * Moved printer.name to the next line so that it won't break the 80 character limit.

--

Changelog:

2015-03-11  Martin Galvan  <martin.galvan@tallertechnologies.com>

	* python/lib/gdb/printing.py: Fix exception when registering a global pretty-printer in verbose mode.
  

Comments

Doug Evans March 11, 2015, 11:52 p.m. UTC | #1
Martin Galvan writes:
 > This patch fixes a Python exception that was being thrown when trying to register a global pretty-printer with verbose mode on:
 > 
 >   File "/usr/share/gdb/python/gdb/printing.py", line 119, in register_pretty_printer
 >     gdb.write("Registering global %s pretty-printer ...\n" % name)
 > NameError: name 'name' is not defined
 > 
 > My copyright assignment is on the works, but since this is a small patch I don't think it's necessary.
 > 
 > Changes from v1:
 >  * Moved printer.name to the next line so that it won't break the 80 character limit.
 > 
 > --
 > 
 > Changelog:
 > 
 > 2015-03-11  Martin Galvan  <martin.galvan@tallertechnologies.com>
 > 
 > 	* python/lib/gdb/printing.py: Fix exception when registering a global pretty-printer in verbose mode.
 > 
 > diff --git a/gdb/python/lib/gdb/printing.py b/gdb2/python/lib/gdb/printing.py
 > index 47742a9..c935333 100644
 > --- a/gdb/python/lib/gdb/printing.py
 > +++ b/gdb/python/lib/gdb/printing.py
 > @@ -116,7 +116,8 @@ def register_pretty_printer(obj, printer, replace=False):
 > 
 >      if obj is None:
 >          if gdb.parameter("verbose"):
 > -            gdb.write("Registering global %s pretty-printer ...\n" % name)
 > +            gdb.write("Registering global %s pretty-printer ...\n" %
 > +                      printer.name)
 >          obj = gdb
 >      else:
 >          if gdb.parameter("verbose"):

LGTM, with two more nits.
The ChangeLog entry > 80 char limit :-),
and convention is to include the function name in the ChangeLog entry.

E.g.,

2015-03-11  Martin Galvan  <martin.galvan@tallertechnologies.com>

	* python/lib/gdb/printing.py (register_pretty_printer): Fix exception
	when registering a global pretty-printer in verbose mode.

No need to repost though.
Ok to commit with that fixed.
  
Martin Galvan March 12, 2015, 12:24 a.m. UTC | #2
On Wed, Mar 11, 2015 at 8:52 PM, Doug Evans <dje@google.com> wrote:
> LGTM, with two more nits.
> The ChangeLog entry > 80 char limit :-),
> and convention is to include the function name in the ChangeLog entry.
>
> E.g.,
>
> 2015-03-11  Martin Galvan  <martin.galvan@tallertechnologies.com>
>
>         * python/lib/gdb/printing.py (register_pretty_printer): Fix exception
>         when registering a global pretty-printer in verbose mode.
>
> No need to repost though.
> Ok to commit with that fixed.

Thanks a lot! I'll keep that in mind for any future patches.
  
Martin Galvan March 28, 2015, 8:39 p.m. UTC | #3
On Wed, Mar 11, 2015 at 8:52 PM, Doug Evans <dje@google.com> wrote:
> Martin Galvan writes:
>  > This patch fixes a Python exception that was being thrown when trying to register a global pretty-printer with verbose mode on:
>  >
>  >   File "/usr/share/gdb/python/gdb/printing.py", line 119, in register_pretty_printer
>  >     gdb.write("Registering global %s pretty-printer ...\n" % name)
>  > NameError: name 'name' is not defined
>  >
>  > My copyright assignment is on the works, but since this is a small patch I don't think it's necessary.
>  >
>  > Changes from v1:
>  >  * Moved printer.name to the next line so that it won't break the 80 character limit.
>  >
>  > --
>  >
>  > Changelog:
>  >
>  > 2015-03-11  Martin Galvan  <martin.galvan@tallertechnologies.com>
>  >
>  >      * python/lib/gdb/printing.py: Fix exception when registering a global pretty-printer in verbose mode.
>  >
>  > diff --git a/gdb/python/lib/gdb/printing.py b/gdb2/python/lib/gdb/printing.py
>  > index 47742a9..c935333 100644
>  > --- a/gdb/python/lib/gdb/printing.py
>  > +++ b/gdb/python/lib/gdb/printing.py
>  > @@ -116,7 +116,8 @@ def register_pretty_printer(obj, printer, replace=False):
>  >
>  >      if obj is None:
>  >          if gdb.parameter("verbose"):
>  > -            gdb.write("Registering global %s pretty-printer ...\n" % name)
>  > +            gdb.write("Registering global %s pretty-printer ...\n" %
>  > +                      printer.name)
>  >          obj = gdb
>  >      else:
>  >          if gdb.parameter("verbose"):
>
> LGTM, with two more nits.
> The ChangeLog entry > 80 char limit :-),
> and convention is to include the function name in the ChangeLog entry.
>
> E.g.,
>
> 2015-03-11  Martin Galvan  <martin.galvan@tallertechnologies.com>
>
>         * python/lib/gdb/printing.py (register_pretty_printer): Fix exception
>         when registering a global pretty-printer in verbose mode.
>
> No need to repost though.
> Ok to commit with that fixed.

Sorry to bother, but I don't have write privileges on the repository;
would you be so kind to commit this for me?

Thanks a lot!
  
Joel Brobecker Jan. 1, 2016, 11:32 a.m. UTC | #4
> > 2015-03-11  Martin Galvan  <martin.galvan@tallertechnologies.com>
> >
> >         * python/lib/gdb/printing.py (register_pretty_printer): Fix exception
> >         when registering a global pretty-printer in verbose mode.
> >
> > No need to repost though.
> > Ok to commit with that fixed.
> 
> Sorry to bother, but I don't have write privileges on the repository;
> would you be so kind to commit this for me?

Martin - I am going through my gdb-patches mailbox, and I am wondering
if anyone pushed that patch for you? If not, would you mind rebasing
your patch and sending it to us, please (with our sincerest apologies!)?
  
Martin Galvan Jan. 2, 2016, 1:08 a.m. UTC | #5
On Fri, Jan 1, 2016 at 8:32 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> Martin - I am going through my gdb-patches mailbox, and I am wondering
> if anyone pushed that patch for you? If not, would you mind rebasing
> your patch and sending it to us, please (with our sincerest apologies!)?
>
> --
> Joel

Hi! Thanks a lot for writing. I honestly don't remember what happened
to this patch :) however looking at the git master it seems that it's
fixed already. In any case, I now have commit-after-approval access,
so I'll test it next week and commit it myself if it's still breaking.

Again, thanks a lot and happy new year!
  
Joel Brobecker Jan. 2, 2016, 2:46 a.m. UTC | #6
> Hi! Thanks a lot for writing. I honestly don't remember what happened
> to this patch :) however looking at the git master it seems that it's
> fixed already. In any case, I now have commit-after-approval access,
> so I'll test it next week and commit it myself if it's still breaking.

Ah, even better! :)

> Again, thanks a lot and happy new year!

You are very welcome. Happy New Year to you too :).
  

Patch

diff --git a/gdb/python/lib/gdb/printing.py b/gdb2/python/lib/gdb/printing.py
index 47742a9..c935333 100644
--- a/gdb/python/lib/gdb/printing.py
+++ b/gdb/python/lib/gdb/printing.py
@@ -116,7 +116,8 @@  def register_pretty_printer(obj, printer, replace=False):

     if obj is None:
         if gdb.parameter("verbose"):
-            gdb.write("Registering global %s pretty-printer ...\n" % name)
+            gdb.write("Registering global %s pretty-printer ...\n" %
+                      printer.name)
         obj = gdb
     else:
         if gdb.parameter("verbose"):