public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* python unwinder doc improvement
@ 2022-05-28 11:55 Paulo Neves
  2022-05-28 11:55 ` [PATCH 1/2] gdb/python doc: Add register_unwinder to code example Paulo Neves
                   ` (3 more replies)
  0 siblings, 4 replies; 33+ messages in thread
From: Paulo Neves @ 2022-05-28 11:55 UTC (permalink / raw)
  To: gdb-patches

[PATCH 1/2] gdb/python doc: Add register_unwinder to code example
[PATCH 2/2] gdb/python doc: Add enable property to the unwinder

Minor documentation changes to the unwinder example code with the goal
of having all the features described in the prose also reflect on the
code.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 1/2] gdb/python doc: Add register_unwinder to code example
  2022-05-28 11:55 python unwinder doc improvement Paulo Neves
@ 2022-05-28 11:55 ` Paulo Neves
  2022-05-28 12:20   ` Eli Zaretskii
  2022-05-28 11:55 ` [PATCH 2/2] gdb/python doc: Add enable property to the unwinder example Paulo Neves
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-05-28 11:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

It is now clear in the example code that an unwinder needs to be
registered to be used.
---
 gdb/doc/python.texi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index cb5283e03c0..5b7fba798a9 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
         # Return the result:
         return unwind_info
 
+# To use an unwinder it needs to be registered and enabled.
+gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
 @end smallexample
 
 @subheading Registering a Unwinder
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH 2/2] gdb/python doc: Add enable property to the unwinder example
  2022-05-28 11:55 python unwinder doc improvement Paulo Neves
  2022-05-28 11:55 ` [PATCH 1/2] gdb/python doc: Add register_unwinder to code example Paulo Neves
@ 2022-05-28 11:55 ` Paulo Neves
  2022-05-28 12:22   ` Eli Zaretskii
  2022-05-28 13:29 ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Paulo Neves
  2022-06-03 16:01 ` [PATCH v3 1/4] " Paulo Neves
  3 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-05-28 11:55 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

The enable property is now also in the example code making it clear how
it is used and how it influences the operation of an unwinder.
---
 gdb/doc/python.texi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5b7fba798a9..cb148d9b0ea 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2796,6 +2796,10 @@ class FrameId(object):
 class MyUnwinder(Unwinder):
     def __init__(....):
         super(MyUnwinder, self).__init___(<expects unwinder name argument>)
+        # If set to True the unwinder will be enabled.
+        # If upon registration the enable property is True, the unwinder will be usable immediately.
+        # gdb or code may change value.
+        self.enabled = <True|False>
 
     def __call__(pending_frame):
         if not <we recognize frame>:
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example
  2022-05-28 11:55 ` [PATCH 1/2] gdb/python doc: Add register_unwinder to code example Paulo Neves
@ 2022-05-28 12:20   ` Eli Zaretskii
  2022-06-21 11:17     ` Paulo Neves
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2022-05-28 12:20 UTC (permalink / raw)
  To: ptsneves; +Cc: gdb-patches

> Date: Sat, 28 May 2022 13:55:08 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
> 
> It is now clear in the example code that an unwinder needs to be
> registered to be used.
> ---
>  gdb/doc/python.texi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index cb5283e03c0..5b7fba798a9 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
>          # Return the result:
>          return unwind_info
>  
> +# To use an unwinder it needs to be registered and enabled.
> +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
>  @end smallexample

Thanks, I think this needs to be reviewed by someone who knows how to
program the Python unwinders.  As far as Texinfo is concerned, this is
fine.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] gdb/python doc: Add enable property to the unwinder example
  2022-05-28 11:55 ` [PATCH 2/2] gdb/python doc: Add enable property to the unwinder example Paulo Neves
@ 2022-05-28 12:22   ` Eli Zaretskii
  2022-05-28 13:35     ` Paulo Neves
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2022-05-28 12:22 UTC (permalink / raw)
  To: ptsneves; +Cc: gdb-patches

> Date: Sat, 28 May 2022 13:55:09 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
> 
> The enable property is now also in the example code making it clear how
> it is used and how it influences the operation of an unwinder.
> ---
>  gdb/doc/python.texi | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 5b7fba798a9..cb148d9b0ea 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2796,6 +2796,10 @@ class FrameId(object):
>  class MyUnwinder(Unwinder):
>      def __init__(....):
>          super(MyUnwinder, self).__init___(<expects unwinder name argument>)
> +        # If set to True the unwinder will be enabled.
> +        # If upon registration the enable property is True, the unwinder will be usable immediately.

This last line is too long for an @example, please break it into two.

> +        # gdb or code may change value.
> +        self.enabled = <True|False>
>  
>      def __call__(pending_frame):
>          if not <we recognize frame>:

Other than that, this needs to be reviewed by someone who knows about
programming unwinders in Python.

Thanks.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example
  2022-05-28 11:55 python unwinder doc improvement Paulo Neves
  2022-05-28 11:55 ` [PATCH 1/2] gdb/python doc: Add register_unwinder to code example Paulo Neves
  2022-05-28 11:55 ` [PATCH 2/2] gdb/python doc: Add enable property to the unwinder example Paulo Neves
@ 2022-05-28 13:29 ` Paulo Neves
  2022-05-28 13:29   ` [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example Paulo Neves
                     ` (2 more replies)
  2022-06-03 16:01 ` [PATCH v3 1/4] " Paulo Neves
  3 siblings, 3 replies; 33+ messages in thread
From: Paulo Neves @ 2022-05-28 13:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

It is now clear in the example code that an unwinder needs to be
registered to be used.
---
 gdb/doc/python.texi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index cb5283e03c0..5b7fba798a9 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
         # Return the result:
         return unwind_info
 
+# To use an unwinder it needs to be registered and enabled.
+gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
 @end smallexample
 
 @subheading Registering a Unwinder
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example
  2022-05-28 13:29 ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Paulo Neves
@ 2022-05-28 13:29   ` Paulo Neves
  2022-05-30  9:57     ` Andrew Burgess
  2022-05-28 13:29   ` [PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
  2022-05-30  9:55   ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Andrew Burgess
  2 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-05-28 13:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

The enable property is now also in the example code making it clear how
it is used and how it influences the operation of an unwinder.
---
 gdb/doc/python.texi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 5b7fba798a9..444e332d752 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2796,6 +2796,11 @@ class FrameId(object):
 class MyUnwinder(Unwinder):
     def __init__(....):
         super(MyUnwinder, self).__init___(<expects unwinder name argument>)
+        # If set to True the unwinder will be enabled.
+        # If upon registration the enable property is True, the unwinder
+        # will be usable immediately.
+        # gdb or code may change value.
+        self.enabled = <True|False>
 
     def __call__(pending_frame):
         if not <we recognize frame>:
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder
  2022-05-28 13:29 ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Paulo Neves
  2022-05-28 13:29   ` [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example Paulo Neves
@ 2022-05-28 13:29   ` Paulo Neves
  2022-05-30  9:33     ` Andrew Burgess
  2022-05-30  9:55   ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Andrew Burgess
  2 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-05-28 13:29 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does
not and trying to import it leads to errors. Other instances of the
manual correctly refer to gdb.unwinder.
---
 gdb/doc/python.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 444e332d752..59232750099 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2785,7 +2785,7 @@ class.  Derive your unwinder class from it and structure the code as
 follows:
 
 @smallexample
-from gdb.unwinders import Unwinder
+from gdb.unwinder import Unwinder
 
 class FrameId(object):
     def __init__(self, sp, pc):
@@ -2828,7 +2828,7 @@ gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
 An object file, a program space, and the @value{GDBN} proper can have
 unwinders registered with it.
 
-The @code{gdb.unwinders} module provides the function to register a
+The @code{gdb.unwinder} module provides the function to register a
 unwinder:
 
 @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 2/2] gdb/python doc: Add enable property to the unwinder example
  2022-05-28 12:22   ` Eli Zaretskii
@ 2022-05-28 13:35     ` Paulo Neves
  0 siblings, 0 replies; 33+ messages in thread
From: Paulo Neves @ 2022-05-28 13:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 5/28/22 14:22, Eli Zaretskii wrote:

>> Date: Sat, 28 May 2022 13:55:09 +0200
>> From: Paulo Neves via Gdb-patches<gdb-patches@sourceware.org>
>> Cc: Paulo Neves<ptsneves@gmail.com>
>>
>> The enable property is now also in the example code making it clear how
>> it is used and how it influences the operation of an unwinder.
>> ---
>>   gdb/doc/python.texi | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index 5b7fba798a9..cb148d9b0ea 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -2796,6 +2796,10 @@ class FrameId(object):
>>   class MyUnwinder(Unwinder):
>>       def __init__(....):
>>           super(MyUnwinder, self).__init___(<expects unwinder name argument>)
>> +        # If set to True the unwinder will be enabled.
>> +        # If upon registration the enable property is True, the unwinder will be usable immediately.
> This last line is too long for an @example, please break it into two.
Done
>
>> +        # gdb or code may change value.
>> +        self.enabled = <True|False>
>>   
>>       def __call__(pending_frame):
>>           if not <we recognize frame>:
> Other than that, this needs to be reviewed by someone who knows about
> programming unwinders in Python.
>
> Thanks.
Ok, I will be waiting then and ping according to the guidelines. I added 
a 3rd patch that also corrects a mistake in the module name mentioned in 
the example and doc.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder
  2022-05-28 13:29   ` [PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
@ 2022-05-30  9:33     ` Andrew Burgess
  0 siblings, 0 replies; 33+ messages in thread
From: Andrew Burgess @ 2022-05-30  9:33 UTC (permalink / raw)
  To: Paulo Neves via Gdb-patches, gdb-patches; +Cc: Paulo Neves

Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes:

> Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does
> not and trying to import it leads to errors. Other instances of the
> manual correctly refer to gdb.unwinder.

This patch is fine.

Thanks,
Andrew

> ---
>  gdb/doc/python.texi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 444e332d752..59232750099 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2785,7 +2785,7 @@ class.  Derive your unwinder class from it and structure the code as
>  follows:
>  
>  @smallexample
> -from gdb.unwinders import Unwinder
> +from gdb.unwinder import Unwinder
>  
>  class FrameId(object):
>      def __init__(self, sp, pc):
> @@ -2828,7 +2828,7 @@ gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
>  An object file, a program space, and the @value{GDBN} proper can have
>  unwinders registered with it.
>  
> -The @code{gdb.unwinders} module provides the function to register a
> +The @code{gdb.unwinder} module provides the function to register a
>  unwinder:
>  
>  @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
> -- 
> 2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example
  2022-05-28 13:29 ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Paulo Neves
  2022-05-28 13:29   ` [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example Paulo Neves
  2022-05-28 13:29   ` [PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
@ 2022-05-30  9:55   ` Andrew Burgess
  2022-05-30 10:12     ` Paulo Neves
  2 siblings, 1 reply; 33+ messages in thread
From: Andrew Burgess @ 2022-05-30  9:55 UTC (permalink / raw)
  To: Paulo Neves via Gdb-patches, gdb-patches; +Cc: Paulo Neves

Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes:

> It is now clear in the example code that an unwinder needs to be
> registered to be used.
> ---
>  gdb/doc/python.texi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index cb5283e03c0..5b7fba798a9 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
>          # Return the result:
>          return unwind_info
>  
> +# To use an unwinder it needs to be registered and enabled.
> +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
>  @end smallexample

I've had reason to read the unwinder example before, and I think it's
probably my most hated example in our manual.

I absolutely hate that the example is not real, executable, EXAMPLE,
code, it's just seems like documentation in another form.

That said, I think its not fair for me to require that your additions to
the existing example be executable, when the rest of the example is
not.  But, if you _did_ want to improve this whole example, then I'd
certainly be in favour!

I do however, this that with this addition, the example code is now in
the wrong place.  Notice your example now makes use of
`register_unwinder`, but this method is not discussed in detail until
after the example code.

I think that the whole subsection titled `Unwinder Skeleton Code` should
be moved later, probably to just before `@node Xmethods In Python`, that
is, right at the end of the section on unwinders.

Thanks,
Andrew

>  
>  @subheading Registering a Unwinder
> -- 
> 2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example
  2022-05-28 13:29   ` [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example Paulo Neves
@ 2022-05-30  9:57     ` Andrew Burgess
  2022-06-03 15:54       ` Paulo Neves
  0 siblings, 1 reply; 33+ messages in thread
From: Andrew Burgess @ 2022-05-30  9:57 UTC (permalink / raw)
  To: Paulo Neves via Gdb-patches, gdb-patches; +Cc: Paulo Neves

Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes:

> The enable property is now also in the example code making it clear how
> it is used and how it influences the operation of an unwinder.
> ---
>  gdb/doc/python.texi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 5b7fba798a9..444e332d752 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2796,6 +2796,11 @@ class FrameId(object):
>  class MyUnwinder(Unwinder):
>      def __init__(....):
>          super(MyUnwinder, self).__init___(<expects unwinder name argument>)
> +        # If set to True the unwinder will be enabled.
> +        # If upon registration the enable property is True, the unwinder
> +        # will be usable immediately.
> +        # gdb or code may change value.
> +        self.enabled = <True|False>

Everything I said about patch #1 applies here too.

I think your comment could be shortened to just:

  # If set to True the unwinder will be enabled and available for use.

Given that text I think most users will assume that setting the field to
`True` in the constructor will result in the unwinder being available
upon registration.

Thanks,
Andrew


>  
>      def __call__(pending_frame):
>          if not <we recognize frame>:
> -- 
> 2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example
  2022-05-30  9:55   ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Andrew Burgess
@ 2022-05-30 10:12     ` Paulo Neves
  0 siblings, 0 replies; 33+ messages in thread
From: Paulo Neves @ 2022-05-30 10:12 UTC (permalink / raw)
  To: Andrew Burgess, Paulo Neves via Gdb-patches

Hello Andrew

On 5/30/22 11:55, Andrew Burgess wrote:
> Paulo Neves via Gdb-patches<gdb-patches@sourceware.org>  writes:
>
>> It is now clear in the example code that an unwinder needs to be
>> registered to be used.
>> ---
>>   gdb/doc/python.texi | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index cb5283e03c0..5b7fba798a9 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
>>           # Return the result:
>>           return unwind_info
>>   
>> +# To use an unwinder it needs to be registered and enabled.
>> +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
>>   @end smallexample
> I've had reason to read the unwinder example before, and I think it's
> probably my most hated example in our manual.
>
> I absolutely hate that the example is not real, executable, EXAMPLE,
> code, it's just seems like documentation in another form.

I think the fact it was not executable was the reason that nobody
noticed the import was wrong.

>
> That said, I think its not fair for me to require that your additions to
> the existing example be executable, when the rest of the example is
> not.  But, if you _did_ want to improve this whole example, then I'd
> certainly be in favour!

When you mean you want it to be executable, does this mean i should to
remove the meta-values like "<SP number>" sprinkled in the code and
replace them with comments?

Also by real example do you want it to be functional?
I am concerned an example that does any meaningful thing would be confusing.
Something i looked at when trying to get my unwinder working was the
gdb/testsuite/gdb.python/py-unwind.py test. It could be used but adds unrelated logic.

>
> I do however, this that with this addition, the example code is now in
> the wrong place.  Notice your example now makes use of
> `register_unwinder`, but this method is not discussed in detail until
> after the example code.
>
> I think that the whole subsection titled `Unwinder Skeleton Code` should
> be moved later, probably to just before `@node Xmethods In Python`, that
> is, right at the end of the section on unwinders.

I can move the example to the last subsection.

>
> Thanks,
> Andrew
>
>>   
>>   @subheading Registering a Unwinder
>> -- 
>> 2.25.1

I am interested in getting this section improved. Just give me the guidelines to the questions above
and I will prepare the patches.

Paulo Neves

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example
  2022-05-30  9:57     ` Andrew Burgess
@ 2022-06-03 15:54       ` Paulo Neves
  0 siblings, 0 replies; 33+ messages in thread
From: Paulo Neves @ 2022-06-03 15:54 UTC (permalink / raw)
  To: Andrew Burgess, Paulo Neves via Gdb-patches



On 5/30/22 11:57, Andrew Burgess wrote:
> Paulo Neves via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> The enable property is now also in the example code making it clear how
>> it is used and how it influences the operation of an unwinder.
>> ---
>>   gdb/doc/python.texi | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index 5b7fba798a9..444e332d752 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -2796,6 +2796,11 @@ class FrameId(object):
>>   class MyUnwinder(Unwinder):
>>       def __init__(....):
>>           super(MyUnwinder, self).__init___(<expects unwinder name argument>)
>> +        # If set to True the unwinder will be enabled.
>> +        # If upon registration the enable property is True, the unwinder
>> +        # will be usable immediately.
>> +        # gdb or code may change value.
>> +        self.enabled = <True|False>
> Everything I said about patch #1 applies here too.
>
> I think your comment could be shortened to just:
>
>    # If set to True the unwinder will be enabled and available for use.
>
> Given that text I think most users will assume that setting the field to
> `True` in the constructor will result in the unwinder being available
> upon registration.
I think that part can be indeed removed but I think the hint that gdb 
may change values of your unwinder is important. I wrote these changes 
though the eyes of the difficulties I had as an absolute GDB extension 
newbie.

> Thanks,
> Andrew
>
>
>>   
>>       def __call__(pending_frame):
>>           if not <we recognize frame>:
>> -- 
>> 2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 1/4] gdb/python doc: Add register_unwinder to code example
  2022-05-28 11:55 python unwinder doc improvement Paulo Neves
                   ` (2 preceding siblings ...)
  2022-05-28 13:29 ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Paulo Neves
@ 2022-06-03 16:01 ` Paulo Neves
  2022-06-03 16:01   ` [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example Paulo Neves
                     ` (3 more replies)
  3 siblings, 4 replies; 33+ messages in thread
From: Paulo Neves @ 2022-06-03 16:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

It is now clear in the example code that an unwinder needs to be
registered to be used.
---
 gdb/doc/python.texi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index ba5a9b315e1..228d515a817 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2823,6 +2823,8 @@ class MyUnwinder(Unwinder):
         # Return the result:
         return unwind_info
 
+# To use an unwinder it needs to be registered and enabled.
+gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
 @end smallexample
 
 @subheading Registering a Unwinder
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example
  2022-06-03 16:01 ` [PATCH v3 1/4] " Paulo Neves
@ 2022-06-03 16:01   ` Paulo Neves
  2022-06-03 16:17     ` Eli Zaretskii
  2022-06-03 16:01   ` [PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-06-03 16:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

The enable property is now also in the example code making it clear how
it is used and how it influences the operation of an unwinder.
---
 gdb/doc/python.texi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 228d515a817..3e4833a4a41 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2805,6 +2805,9 @@ class FrameId(object):
 class MyUnwinder(Unwinder):
     def __init__(....):
         super(MyUnwinder, self).__init___(<expects unwinder name argument>)
+        # If set to True the unwinder will be enabled.
+        # gdb or code may change this value.
+        self.enabled = <True|False>
 
     def __call__(pending_frame):
         if not <we recognize frame>:
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder
  2022-06-03 16:01 ` [PATCH v3 1/4] " Paulo Neves
  2022-06-03 16:01   ` [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example Paulo Neves
@ 2022-06-03 16:01   ` Paulo Neves
  2022-06-03 16:17     ` Eli Zaretskii
  2022-06-03 16:01   ` [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code Paulo Neves
  2022-06-03 16:17   ` [PATCH v3 1/4] gdb/python doc: Add register_unwinder to code example Eli Zaretskii
  3 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-06-03 16:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does
not and trying to import it leads to errors. Other instances of the
manual correctly refer to gdb.unwinder.
---
 gdb/doc/python.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 3e4833a4a41..e2821755851 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2794,7 +2794,7 @@ class.  Derive your unwinder class from it and structure the code as
 follows:
 
 @smallexample
-from gdb.unwinders import Unwinder
+from gdb.unwinder import Unwinder
 
 class FrameId(object):
     def __init__(self, sp, pc):
@@ -2835,7 +2835,7 @@ gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
 An object file, a program space, and the @value{GDBN} proper can have
 unwinders registered with it.
 
-The @code{gdb.unwinders} module provides the function to register a
+The @code{gdb.unwinder} module provides the function to register a
 unwinder:
 
 @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-06-03 16:01 ` [PATCH v3 1/4] " Paulo Neves
  2022-06-03 16:01   ` [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example Paulo Neves
  2022-06-03 16:01   ` [PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
@ 2022-06-03 16:01   ` Paulo Neves
  2022-06-03 16:20     ` Eli Zaretskii
  2022-06-03 16:17   ` [PATCH v3 1/4] gdb/python doc: Add register_unwinder to code example Eli Zaretskii
  3 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-06-03 16:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Paulo Neves

This way all the concepts mentioned in the skeleton code are
previously detailed.
---
 gdb/doc/python.texi | 50 ++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index e2821755851..1deeae82b9b 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2787,6 +2787,31 @@ values see @ref{gdbpy_frame_read_register,,Frame.read_register}.
 @var{value} is a register value (a @code{gdb.Value} object).
 @end defun
 
+@subheading Registering a Unwinder
+
+An object file, a program space, and the @value{GDBN} proper can have
+unwinders registered with it.
+
+The @code{gdb.unwinder} module provides the function to register a
+unwinder:
+
+@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
+@var{locus} is specifies an object file or a program space to which
+@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
+@var{unwinder} to the @value{GDBN}'s global unwinder list.  The newly
+added @var{unwinder} will be called before any other unwinder from the
+same locus.  Two unwinders in the same locus cannot have the same
+name.  An attempt to add a unwinder with already existing name raises
+an exception unless @var{replace} is @code{True}, in which case the
+old unwinder is deleted.
+@end defun
+
+@subheading Unwinder Precedence
+
+@value{GDBN} first calls the unwinders from all the object files in no
+particular order, then the unwinders from the current program space,
+and finally the unwinders from @value{GDBN}.
+
 @subheading Unwinder Skeleton Code
 
 @value{GDBN} comes with the module containing the base @code{Unwinder}
@@ -2830,31 +2855,6 @@ class MyUnwinder(Unwinder):
 gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
 @end smallexample
 
-@subheading Registering a Unwinder
-
-An object file, a program space, and the @value{GDBN} proper can have
-unwinders registered with it.
-
-The @code{gdb.unwinder} module provides the function to register a
-unwinder:
-
-@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
-@var{locus} is specifies an object file or a program space to which
-@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
-@var{unwinder} to the @value{GDBN}'s global unwinder list.  The newly
-added @var{unwinder} will be called before any other unwinder from the
-same locus.  Two unwinders in the same locus cannot have the same
-name.  An attempt to add a unwinder with already existing name raises
-an exception unless @var{replace} is @code{True}, in which case the
-old unwinder is deleted.
-@end defun
-
-@subheading Unwinder Precedence
-
-@value{GDBN} first calls the unwinders from all the object files in no
-particular order, then the unwinders from the current program space,
-and finally the unwinders from @value{GDBN}.
-
 @node Xmethods In Python
 @subsubsection Xmethods In Python
 @cindex xmethods in Python
-- 
2.25.1


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 1/4] gdb/python doc: Add register_unwinder to code example
  2022-06-03 16:01 ` [PATCH v3 1/4] " Paulo Neves
                     ` (2 preceding siblings ...)
  2022-06-03 16:01   ` [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code Paulo Neves
@ 2022-06-03 16:17   ` Eli Zaretskii
  3 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2022-06-03 16:17 UTC (permalink / raw)
  To: Paulo Neves; +Cc: gdb-patches

> Date: Fri,  3 Jun 2022 18:01:36 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
> 
> It is now clear in the example code that an unwinder needs to be
> registered to be used.
> ---
>  gdb/doc/python.texi | 2 ++
>  1 file changed, 2 insertions(+)

OK, thanks.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example
  2022-06-03 16:01   ` [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example Paulo Neves
@ 2022-06-03 16:17     ` Eli Zaretskii
  0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2022-06-03 16:17 UTC (permalink / raw)
  To: Paulo Neves; +Cc: gdb-patches

> Date: Fri,  3 Jun 2022 18:01:37 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
> 
> The enable property is now also in the example code making it clear how
> it is used and how it influences the operation of an unwinder.
> ---
>  gdb/doc/python.texi | 3 +++
>  1 file changed, 3 insertions(+)

This is okay, thanks.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder
  2022-06-03 16:01   ` [PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
@ 2022-06-03 16:17     ` Eli Zaretskii
  0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2022-06-03 16:17 UTC (permalink / raw)
  To: Paulo Neves; +Cc: gdb-patches

> Date: Fri,  3 Jun 2022 18:01:38 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
> 
> Replace gdb.unwinders with gdb.unwinder. The gdb.unwinders module does
> not and trying to import it leads to errors. Other instances of the
> manual correctly refer to gdb.unwinder.
> ---
>  gdb/doc/python.texi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

OK, thanks.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-06-03 16:01   ` [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code Paulo Neves
@ 2022-06-03 16:20     ` Eli Zaretskii
  2022-06-03 16:38       ` Paulo Neves
  2022-09-30 14:50       ` Pedro Alves
  0 siblings, 2 replies; 33+ messages in thread
From: Eli Zaretskii @ 2022-06-03 16:20 UTC (permalink / raw)
  To: Paulo Neves; +Cc: gdb-patches

> Date: Fri,  3 Jun 2022 18:01:39 +0200
> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
> Cc: Paulo Neves <ptsneves@gmail.com>
> 
> +@subheading Registering a Unwinder
> +
> +An object file, a program space, and the @value{GDBN} proper can have
> +unwinders registered with it.
> +
> +The @code{gdb.unwinder} module provides the function to register a
> +unwinder:
> +
> +@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
> +@var{locus} is specifies an object file or a program space to which
               ^^
The "is" part should be deleted.

> +@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
> +@var{unwinder} to the @value{GDBN}'s global unwinder list.

First, @value{GDBN}, not @code{gdb}.

And also, this sentence doesn't read well ("passing None or gdb
adds..."), I guess it's some typo or missing or redundant word.

Thanks.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-06-03 16:20     ` Eli Zaretskii
@ 2022-06-03 16:38       ` Paulo Neves
  2022-09-30 14:50       ` Pedro Alves
  1 sibling, 0 replies; 33+ messages in thread
From: Paulo Neves @ 2022-06-03 16:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 6/3/22 18:20, Eli Zaretskii wrote:
>> Date: Fri,  3 Jun 2022 18:01:39 +0200
>> From: Paulo Neves via Gdb-patches<gdb-patches@sourceware.org>
>> Cc: Paulo Neves<ptsneves@gmail.com>
>>
>> +@subheading Registering a Unwinder
>> +
>> +An object file, a program space, and the @value{GDBN} proper can have
>> +unwinders registered with it.
>> +
>> +The @code{gdb.unwinder} module provides the function to register a
>> +unwinder:
>> +
>> +@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
>> +@var{locus} is specifies an object file or a program space to which
>                 ^^
> The "is" part should be deleted.
>
>> +@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
>> +@var{unwinder} to the @value{GDBN}'s global unwinder list.
> First, @value{GDBN}, not @code{gdb}.
>
> And also, this sentence doesn't read well ("passing None or gdb
> adds..."), I guess it's some typo or missing or redundant word.
>
> Thanks.

This patch is purely a move from one place to another, as suggested by Andrew.
The issues you mention are not introduced in this patch, but i will try to
fix the issues you mention on another commit and then rebase this patch on top of it.

Paulo Neves


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example
  2022-05-28 12:20   ` Eli Zaretskii
@ 2022-06-21 11:17     ` Paulo Neves
  2022-06-21 12:19       ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-06-21 11:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Hello all,

Any idea when this can be merged or if there anything missing?

Paulo Neves

On 5/28/22 14:20, Eli Zaretskii wrote:
>> Date: Sat, 28 May 2022 13:55:08 +0200
>> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Paulo Neves <ptsneves@gmail.com>
>>
>> It is now clear in the example code that an unwinder needs to be
>> registered to be used.
>> ---
>>   gdb/doc/python.texi | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index cb5283e03c0..5b7fba798a9 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -2814,6 +2814,8 @@ class MyUnwinder(Unwinder):
>>           # Return the result:
>>           return unwind_info
>>   
>> +# To use an unwinder it needs to be registered and enabled.
>> +gdb.unwinder.register_unwinder (<locus>, MyUnwinder, replace=<True|False>)
>>   @end smallexample
> Thanks, I think this needs to be reviewed by someone who knows how to
> program the Python unwinders.  As far as Texinfo is concerned, this is
> fine.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example
  2022-06-21 11:17     ` Paulo Neves
@ 2022-06-21 12:19       ` Eli Zaretskii
  2022-09-30 11:59         ` Paulo Neves
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2022-06-21 12:19 UTC (permalink / raw)
  To: Paulo Neves; +Cc: gdb-patches

> Date: Tue, 21 Jun 2022 13:17:32 +0200
> Cc: gdb-patches@sourceware.org
> From: Paulo Neves <ptsneves@gmail.com>
> 
> Hello all,
> 
> Any idea when this can be merged or if there anything missing?

Like I said before: this needs to be approved by someone who knows
about Python unwinder programming.  If the text is correct, the
Texinfo use was already approved by me.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example
  2022-06-21 12:19       ` Eli Zaretskii
@ 2022-09-30 11:59         ` Paulo Neves
  2022-09-30 12:25           ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: Paulo Neves @ 2022-09-30 11:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 6/21/22 14:19, Eli Zaretskii wrote:
>> Date: Tue, 21 Jun 2022 13:17:32 +0200
>> Cc: gdb-patches@sourceware.org
>> From: Paulo Neves <ptsneves@gmail.com>
>>
>> Hello all,
>>
>> Any idea when this can be merged or if there anything missing?
> Like I said before: this needs to be approved by someone who knows
> about Python unwinder programming.  If the text is correct, the
> Texinfo use was already approved by me.
Hello Eli,
Could you point me out somebody who knows the python unwinder part that 
i can ping?

Paulo Neves

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example
  2022-09-30 11:59         ` Paulo Neves
@ 2022-09-30 12:25           ` Eli Zaretskii
  0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2022-09-30 12:25 UTC (permalink / raw)
  To: Paulo Neves; +Cc: gdb-patches

> Date: Fri, 30 Sep 2022 13:59:06 +0200
> Cc: gdb-patches@sourceware.org
> From: Paulo Neves <ptsneves@gmail.com>
> 
> On 6/21/22 14:19, Eli Zaretskii wrote:
> >> Date: Tue, 21 Jun 2022 13:17:32 +0200
> >> Cc: gdb-patches@sourceware.org
> >> From: Paulo Neves <ptsneves@gmail.com>
> >>
> >> Hello all,
> >>
> >> Any idea when this can be merged or if there anything missing?
> > Like I said before: this needs to be approved by someone who knows
> > about Python unwinder programming.  If the text is correct, the
> > Texinfo use was already approved by me.
> Hello Eli,
> Could you point me out somebody who knows the python unwinder part that 
> i can ping?

Any other of the global maintainers, really: Joel, Pedro, Simon, and
many others.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-06-03 16:20     ` Eli Zaretskii
  2022-06-03 16:38       ` Paulo Neves
@ 2022-09-30 14:50       ` Pedro Alves
  2022-09-30 15:53         ` Eli Zaretskii
  1 sibling, 1 reply; 33+ messages in thread
From: Pedro Alves @ 2022-09-30 14:50 UTC (permalink / raw)
  To: Eli Zaretskii, Paulo Neves; +Cc: gdb-patches

On 2022-06-03 5:20 p.m., Eli Zaretskii via Gdb-patches wrote:
>> Date: Fri,  3 Jun 2022 18:01:39 +0200
>> From: Paulo Neves via Gdb-patches <gdb-patches@sourceware.org>
>> Cc: Paulo Neves <ptsneves@gmail.com>
>>
>> +@subheading Registering a Unwinder
>> +
>> +An object file, a program space, and the @value{GDBN} proper can have
>> +unwinders registered with it.
>> +
>> +The @code{gdb.unwinder} module provides the function to register a
>> +unwinder:
>> +
>> +@defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
>> +@var{locus} is specifies an object file or a program space to which
>                ^^
> The "is" part should be deleted.
> 
>> +@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
>> +@var{unwinder} to the @value{GDBN}'s global unwinder list.
> 
> First, @value{GDBN}, not @code{gdb}.
> 
> And also, this sentence doesn't read well ("passing None or gdb
> adds..."), I guess it's some typo or missing or redundant word.
> 

I think the intent is to say that you can pass literal "None" or "gdb"
to gdb.unwinder.register_unwinder, for "locus" argument.  Reading like that,
it makes sense to me, and replacing with @value{GDBN} would be incorrect.


However, looking at the code itself, I don't see "gdb" being accepted as input:

From gdb/python/lib/gdb/unwinder.py :

def register_unwinder(locus, unwinder, replace=False):
    """Register unwinder in given locus.

    The unwinder is prepended to the locus's unwinders list. Unwinder
    name should be unique.

    Arguments:
        locus: Either an objfile, progspace, or None (in which case
               the unwinder is registered globally).
        unwinder: An object of a gdb.Unwinder subclass
        replace: If True, replaces existing unwinder with the same name.
                 Otherwise, raises exception if unwinder with the same
                 name already exists.

    Returns:
        Nothing.

    Raises:
        RuntimeError: Unwinder name is not unique
        TypeError: Bad locus type
    """
    if locus is None:
        if gdb.parameter("verbose"):
            gdb.write("Registering global %s unwinder ...\n" % unwinder.name)
        locus = gdb
    elif isinstance(locus, gdb.Objfile) or isinstance(locus, gdb.Progspace):
        if gdb.parameter("verbose"):
            gdb.write(
                "Registering %s unwinder for %s ...\n" % (unwinder.name, locus.filename)
            )
    else:
        raise TypeError("locus should be gdb.Objfile or gdb.Progspace or None")

    i = 0
    for needle in locus.frame_unwinders:
        if needle.name == unwinder.name:
            if replace:
                del locus.frame_unwinders[i]
            else:
                raise RuntimeError("Unwinder %s already exists." % unwinder.name)
        i += 1
    locus.frame_unwinders.insert(0, unwinder)
    gdb.invalidate_cached_frames()



Above, when you pass "None", locus does end up being "gdb", 

    if locus is None:
        if gdb.parameter("verbose"):
            gdb.write("Registering global %s unwinder ...\n" % unwinder.name)
        locus = gdb
        ^^^^^^^^^^^

but, if you pass locus=gdb as argument, seems to me you'll reach the else branch:

    else:
        raise TypeError("locus should be gdb.Objfile or gdb.Progspace or None")

And note that error string does not mention the "gdb" possibility.


So I think the manual should be fixed by dropping the "or @code{gdb}", resulting in:

  Passing @code{None} adds @var{unwinder} to the @value{GDBN}'s global unwinder list.

Actually, the "the" in "to the @value{GDBN}'s" looks wrong to me.  So I think we
should really say:

  Passing @code{None} adds @var{unwinder} to @value{GDBN}'s global unwinder list.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-09-30 14:50       ` Pedro Alves
@ 2022-09-30 15:53         ` Eli Zaretskii
  2022-09-30 16:41           ` Pedro Alves
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2022-09-30 15:53 UTC (permalink / raw)
  To: Pedro Alves; +Cc: ptsneves, gdb-patches

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 30 Sep 2022 15:50:20 +0100
> 
> So I think the manual should be fixed by dropping the "or @code{gdb}", resulting in:
> 
>   Passing @code{None} adds @var{unwinder} to the @value{GDBN}'s global unwinder list.
> 
> Actually, the "the" in "to the @value{GDBN}'s" looks wrong to me.  So I think we
> should really say:
> 
>   Passing @code{None} adds @var{unwinder} to @value{GDBN}'s global unwinder list.

How about

  If @var{locus} is @code{None}, that stands for @value{GDBN}'s global
  unwinder list.

I think this is clearer and less likely to cause misinterpretation.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-09-30 15:53         ` Eli Zaretskii
@ 2022-09-30 16:41           ` Pedro Alves
  2022-09-30 17:37             ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: Pedro Alves @ 2022-09-30 16:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ptsneves, gdb-patches

On 2022-09-30 4:53 p.m., Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>> Date: Fri, 30 Sep 2022 15:50:20 +0100
>>
>> So I think the manual should be fixed by dropping the "or @code{gdb}", resulting in:
>>
>>   Passing @code{None} adds @var{unwinder} to the @value{GDBN}'s global unwinder list.
>>
>> Actually, the "the" in "to the @value{GDBN}'s" looks wrong to me.  So I think we
>> should really say:
>>
>>   Passing @code{None} adds @var{unwinder} to @value{GDBN}'s global unwinder list.
> 
> How about
> 
>   If @var{locus} is @code{None}, that stands for @value{GDBN}'s global
>   unwinder list.
> 
> I think this is clearer and less likely to cause misinterpretation.
> 

Seems fine in isolation, but looking at the whole paragraph, how about similar text to
what's in gdb/python/lib/gdb/unwinder.py?  Seems clear enough to me.

I also noticed a couple instances of "a unwinder", also fixed.

While at it, "GDB proper" seems like a weird conception.

How about this?

From 7c5e7875bed7f4316823db67d95821c88c679daf Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Fri, 30 Sep 2022 17:23:03 +0100
Subject: [PATCH] Python

Change-Id: I98c1b1000e1063815238e945ca71ec6f37b5702e
---
 gdb/doc/python.texi | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index 2692211f388..4a7b047c3e0 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder):
 
 @end smallexample
 
-@subheading Registering a Unwinder
+@subheading Registering an Unwinder
 
-An object file, a program space, and the @value{GDBN} proper can have
-unwinders registered with it.
+Object files and program spaces can have unwinders registered with
+them.  In addition, you can also register unwinders globally.
 
-The @code{gdb.unwinders} module provides the function to register a
+The @code{gdb.unwinders} module provides the function to register an
 unwinder:
 
 @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
-@var{locus} is specifies an object file or a program space to which
-@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
-@var{unwinder} to the @value{GDBN}'s global unwinder list.  The newly
-added @var{unwinder} will be called before any other unwinder from the
+@var{locus} specifies which unwinder list the @var{unwinder} is
+prepended to.  It can be either an object file, a program space, or
+@code{None}, in which case the unwinder is registered globally.  The
+newly added @var{unwinder} will be called before any other unwinder from the
 same locus.  Two unwinders in the same locus cannot have the same
-name.  An attempt to add a unwinder with already existing name raises
+name.  An attempt to add an unwinder with an already existing name raises
 an exception unless @var{replace} is @code{True}, in which case the
 old unwinder is deleted.
 @end defun

base-commit: cfc0ffd31e90972296b81d362a7d83eb60eb21c0
-- 
2.36.0


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-09-30 16:41           ` Pedro Alves
@ 2022-09-30 17:37             ` Eli Zaretskii
  2022-10-21 21:07               ` Pedro Alves
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2022-09-30 17:37 UTC (permalink / raw)
  To: Pedro Alves; +Cc: ptsneves, gdb-patches

> Cc: ptsneves@gmail.com, gdb-patches@sourceware.org
> From: Pedro Alves <pedro@palves.net>
> Date: Fri, 30 Sep 2022 17:41:50 +0100
> 
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 2692211f388..4a7b047c3e0 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder):
>  
>  @end smallexample
>  
> -@subheading Registering a Unwinder
> +@subheading Registering an Unwinder
>  
> -An object file, a program space, and the @value{GDBN} proper can have
> -unwinders registered with it.
> +Object files and program spaces can have unwinders registered with
> +them.  In addition, you can also register unwinders globally.
>  
> -The @code{gdb.unwinders} module provides the function to register a
> +The @code{gdb.unwinders} module provides the function to register an
>  unwinder:
>  
>  @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
> -@var{locus} is specifies an object file or a program space to which
> -@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
> -@var{unwinder} to the @value{GDBN}'s global unwinder list.  The newly
> -added @var{unwinder} will be called before any other unwinder from the
> +@var{locus} specifies which unwinder list the @var{unwinder} is
> +prepended to.  It can be either an object file, a program space, or
> +@code{None}, in which case the unwinder is registered globally.  The
> +newly added @var{unwinder} will be called before any other unwinder from the
>  same locus.  Two unwinders in the same locus cannot have the same
> -name.  An attempt to add a unwinder with already existing name raises
> +name.  An attempt to add an unwinder with an already existing name raises
>  an exception unless @var{replace} is @code{True}, in which case the
>  old unwinder is deleted.
>  @end defun

That's fine, but let's avoid passive tense where we can (and make the
sentence easier to grasp while at that).  So:

 @var{locus} specifies to which unwinder list to prepend the
 @var{unwinder}.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-09-30 17:37             ` Eli Zaretskii
@ 2022-10-21 21:07               ` Pedro Alves
  2022-10-22  6:41                 ` Paulo Neves
  0 siblings, 1 reply; 33+ messages in thread
From: Pedro Alves @ 2022-10-21 21:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ptsneves, gdb-patches

Hi,

On 2022-09-30 6:37 p.m., Eli Zaretskii wrote:
>> Cc: ptsneves@gmail.com, gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>> Date: Fri, 30 Sep 2022 17:41:50 +0100
>>
>> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
>> index 2692211f388..4a7b047c3e0 100644
>> --- a/gdb/doc/python.texi
>> +++ b/gdb/doc/python.texi
>> @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder):
>>  
>>  @end smallexample
>>  
>> -@subheading Registering a Unwinder
>> +@subheading Registering an Unwinder
>>  
>> -An object file, a program space, and the @value{GDBN} proper can have
>> -unwinders registered with it.
>> +Object files and program spaces can have unwinders registered with
>> +them.  In addition, you can also register unwinders globally.
>>  
>> -The @code{gdb.unwinders} module provides the function to register a
>> +The @code{gdb.unwinders} module provides the function to register an
>>  unwinder:
>>  
>>  @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
>> -@var{locus} is specifies an object file or a program space to which
>> -@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
>> -@var{unwinder} to the @value{GDBN}'s global unwinder list.  The newly
>> -added @var{unwinder} will be called before any other unwinder from the
>> +@var{locus} specifies which unwinder list the @var{unwinder} is
>> +prepended to.  It can be either an object file, a program space, or
>> +@code{None}, in which case the unwinder is registered globally.  The
>> +newly added @var{unwinder} will be called before any other unwinder from the
>>  same locus.  Two unwinders in the same locus cannot have the same
>> -name.  An attempt to add a unwinder with already existing name raises
>> +name.  An attempt to add an unwinder with an already existing name raises
>>  an exception unless @var{replace} is @code{True}, in which case the
>>  old unwinder is deleted.
>>  @end defun
> 
> That's fine, but let's avoid passive tense where we can (and make the
> sentence easier to grasp while at that).  So:
> 
>  @var{locus} specifies to which unwinder list to prepend the
>  @var{unwinder}.
> 

Finally done, and merged.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code
  2022-10-21 21:07               ` Pedro Alves
@ 2022-10-22  6:41                 ` Paulo Neves
  0 siblings, 0 replies; 33+ messages in thread
From: Paulo Neves @ 2022-10-22  6:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches

Thank you very much

Paulo Neves

On Fri, 21 Oct 2022 at 23:07, Pedro Alves <pedro@palves.net> wrote:

> Hi,
>
> On 2022-09-30 6:37 p.m., Eli Zaretskii wrote:
> >> Cc: ptsneves@gmail.com, gdb-patches@sourceware.org
> >> From: Pedro Alves <pedro@palves.net>
> >> Date: Fri, 30 Sep 2022 17:41:50 +0100
> >>
> >> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> >> index 2692211f388..4a7b047c3e0 100644
> >> --- a/gdb/doc/python.texi
> >> +++ b/gdb/doc/python.texi
> >> @@ -2861,21 +2861,21 @@ class MyUnwinder(Unwinder):
> >>
> >>  @end smallexample
> >>
> >> -@subheading Registering a Unwinder
> >> +@subheading Registering an Unwinder
> >>
> >> -An object file, a program space, and the @value{GDBN} proper can have
> >> -unwinders registered with it.
> >> +Object files and program spaces can have unwinders registered with
> >> +them.  In addition, you can also register unwinders globally.
> >>
> >> -The @code{gdb.unwinders} module provides the function to register a
> >> +The @code{gdb.unwinders} module provides the function to register an
> >>  unwinder:
> >>
> >>  @defun gdb.unwinder.register_unwinder (locus, unwinder, replace=False)
> >> -@var{locus} is specifies an object file or a program space to which
> >> -@var{unwinder} is added.  Passing @code{None} or @code{gdb} adds
> >> -@var{unwinder} to the @value{GDBN}'s global unwinder list.  The newly
> >> -added @var{unwinder} will be called before any other unwinder from the
> >> +@var{locus} specifies which unwinder list the @var{unwinder} is
> >> +prepended to.  It can be either an object file, a program space, or
> >> +@code{None}, in which case the unwinder is registered globally.  The
> >> +newly added @var{unwinder} will be called before any other unwinder
> from the
> >>  same locus.  Two unwinders in the same locus cannot have the same
> >> -name.  An attempt to add a unwinder with already existing name raises
> >> +name.  An attempt to add an unwinder with an already existing name
> raises
> >>  an exception unless @var{replace} is @code{True}, in which case the
> >>  old unwinder is deleted.
> >>  @end defun
> >
> > That's fine, but let's avoid passive tense where we can (and make the
> > sentence easier to grasp while at that).  So:
> >
> >  @var{locus} specifies to which unwinder list to prepend the
> >  @var{unwinder}.
> >
>
> Finally done, and merged.
>
> Thanks,
> Pedro Alves
>
>

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2022-10-22  6:41 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-28 11:55 python unwinder doc improvement Paulo Neves
2022-05-28 11:55 ` [PATCH 1/2] gdb/python doc: Add register_unwinder to code example Paulo Neves
2022-05-28 12:20   ` Eli Zaretskii
2022-06-21 11:17     ` Paulo Neves
2022-06-21 12:19       ` Eli Zaretskii
2022-09-30 11:59         ` Paulo Neves
2022-09-30 12:25           ` Eli Zaretskii
2022-05-28 11:55 ` [PATCH 2/2] gdb/python doc: Add enable property to the unwinder example Paulo Neves
2022-05-28 12:22   ` Eli Zaretskii
2022-05-28 13:35     ` Paulo Neves
2022-05-28 13:29 ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Paulo Neves
2022-05-28 13:29   ` [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example Paulo Neves
2022-05-30  9:57     ` Andrew Burgess
2022-06-03 15:54       ` Paulo Neves
2022-05-28 13:29   ` [PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
2022-05-30  9:33     ` Andrew Burgess
2022-05-30  9:55   ` [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example Andrew Burgess
2022-05-30 10:12     ` Paulo Neves
2022-06-03 16:01 ` [PATCH v3 1/4] " Paulo Neves
2022-06-03 16:01   ` [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example Paulo Neves
2022-06-03 16:17     ` Eli Zaretskii
2022-06-03 16:01   ` [PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder Paulo Neves
2022-06-03 16:17     ` Eli Zaretskii
2022-06-03 16:01   ` [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code Paulo Neves
2022-06-03 16:20     ` Eli Zaretskii
2022-06-03 16:38       ` Paulo Neves
2022-09-30 14:50       ` Pedro Alves
2022-09-30 15:53         ` Eli Zaretskii
2022-09-30 16:41           ` Pedro Alves
2022-09-30 17:37             ` Eli Zaretskii
2022-10-21 21:07               ` Pedro Alves
2022-10-22  6:41                 ` Paulo Neves
2022-06-03 16:17   ` [PATCH v3 1/4] gdb/python doc: Add register_unwinder to code example Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).