python unwinder doc improvement

python unwinder doc improvement

[Paulo Neves via 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.

[PATCH 1/2] gdb/python doc: Add register_unwinder to code example

[Paulo Neves via Gdb-patches]

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

Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example

[Eli Zaretskii via 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.

Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example

[Paulo Neves via 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.

Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example

[Eli Zaretskii via 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.

Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example

[Paulo Neves via 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

Re: [PATCH 1/2] gdb/python doc: Add register_unwinder to code example

[Eli Zaretskii via 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.

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

[Paulo Neves via Gdb-patches]

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

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

[Eli Zaretskii via 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.

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

[Paulo Neves via 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.

[PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example

[Paulo Neves via Gdb-patches]

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

[PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example

[Paulo Neves via Gdb-patches]

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

Re: [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example

[Andrew Burgess via Gdb-patches]

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

Re: [PATCH v2 2/3] gdb/python doc: Add enable property to the unwinder example

[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

[PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder

[Paulo Neves via Gdb-patches]

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

Re: [PATCH v2 3/3] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder

[Andrew Burgess via Gdb-patches]

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

Re: [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example

[Andrew Burgess via Gdb-patches]

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

Re: [PATCH v2 1/3] gdb/python doc: Add register_unwinder to code example

[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

[PATCH v3 1/4] gdb/python doc: Add register_unwinder to code example

[Paulo Neves via Gdb-patches]

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

[PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example

[Paulo Neves via Gdb-patches]

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

Re: [PATCH v3 2/4] gdb/python doc: Add enable property to the unwinder example

[Eli Zaretskii via 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.

[PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder

[Paulo Neves via Gdb-patches]

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

Re: [PATCH v3 3/4] gdb/python doc: Fix confusion between gdb.unwinders and gdb.unwinder

[Eli Zaretskii via 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.

[PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Paulo Neves via Gdb-patches]

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

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Eli Zaretskii via 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.

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Paulo Neves via 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

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Pedro Alves]

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.

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Eli Zaretskii via 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.

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Pedro Alves]

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

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Eli Zaretskii via 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}.

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Pedro Alves]

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

Re: [PATCH v3 4/4] gdb/python doc: Move unwinder skeleton code

[Paulo Neves via 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
>
>

Re: [PATCH v3 1/4] gdb/python doc: Add register_unwinder to code example

[Eli Zaretskii via 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.