* [PATCH][PR python/21460] Avoid segfault during Python cleanup
@ 2017-05-25 16:26 paul cannon
2017-06-11 20:59 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: paul cannon @ 2017-05-25 16:26 UTC (permalink / raw)
To: gdb-patches
Rationale for the patch and repro instructions are explained in the bug.
I don't have any copyright assignment on file but this really should be trivial enough to avoid that, I think.
gdb/Changelog:
2017-05-25  paul cannon  <paul@thepaul.org>
python/21460
* python.c (gdbpy_set_quit_flag) Check Py_IsInitialized() before calling PyErr_SetInterrupt(), as Python may be shutting down already.
---
 gdb/python/python.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index be92f36..c6a8c17 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -247,7 +247,10 @@ gdbpy_enter::~gdbpy_enter ()
 static void
 gdbpy_set_quit_flag (const struct extension_language_defn *extlang)
 {
- Â PyErr_SetInterrupt ();
+ Â if (Py_IsInitialized ())
+ Â Â {
+ Â Â Â PyErr_SetInterrupt ();
+ Â Â }
 }
Â
 /* Return true if the quit flag has been set, false otherwise.  */
--Â
2.7.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][PR python/21460] Avoid segfault during Python cleanup
2017-05-25 16:26 [PATCH][PR python/21460] Avoid segfault during Python cleanup paul cannon
@ 2017-06-11 20:59 ` Simon Marchi
2017-06-15 20:30 ` Sergio Durigan Junior
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2017-06-11 20:59 UTC (permalink / raw)
To: paul cannon; +Cc: gdb-patches
Hi Paul,
On 2017-05-25 18:26, paul cannon wrote:
> Rationale for the patch and repro instructions are explained in the
> bug.
Thanks for the patch, and sorry for the little delay in processing it.
I think the code in itself is good. I'll just point out a few things to
fix.
> I don't have any copyright assignment on file but this really should
> be trivial enough to avoid that, I think.
That's what I think too. Do you intend to contribute to GDB regularly?
If so we can get you write access to the repo. If not, I can push the
patch on your behalf (when the issues I mentioned are fixed).
> gdb/Changelog:
> 2017-05-25  paul cannon  <paul@thepaul.org>
Should your name have capital letters :) ?
>
> python/21460
> * python.c (gdbpy_set_quit_flag) Check Py_IsInitialized() before
> calling PyErr_SetInterrupt(), as Python may be shutting down already.
The ChangeLog should only contain "what" changed and not "why". So just
the first part:
Check Py_IsInitialized() before calling PyErr_SetInterrupt().
is sufficient. However, the why should be contained in the commit
message. You did a good job at explaining the problem in the bug you
filed, so I think you could just take that and put it in the commit log
(and massage it a bit if needed).
>
> ---
> Â gdb/python/python.c | 5 ++++-
> Â 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/python/python.c b/gdb/python/python.c
> index be92f36..c6a8c17 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -247,7 +247,10 @@ gdbpy_enter::~gdbpy_enter ()
> Â static void
> Â gdbpy_set_quit_flag (const struct extension_language_defn *extlang)
> Â {
> - Â PyErr_SetInterrupt ();
> + Â if (Py_IsInitialized ())
> + Â Â {
> + Â Â Â PyErr_SetInterrupt ();
> + Â Â }
Drop the braces for a single line body:
if (Py_IsInitialized ())
PyErr_SetInterrupt ();
> Â }
> Â
> Â /* Return true if the quit flag has been set, false otherwise. Â */
> --Â
> 2.7.4
Thanks!
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH][PR python/21460] Avoid segfault during Python cleanup
2017-06-11 20:59 ` Simon Marchi
@ 2017-06-15 20:30 ` Sergio Durigan Junior
0 siblings, 0 replies; 3+ messages in thread
From: Sergio Durigan Junior @ 2017-06-15 20:30 UTC (permalink / raw)
To: Simon Marchi; +Cc: paul cannon, gdb-patches
On Sunday, June 11 2017, Simon Marchi wrote:
>>
>> python/21460
>> * python.c (gdbpy_set_quit_flag) Check Py_IsInitialized() before
>> calling PyErr_SetInterrupt(), as Python may be shutting down already.
>
> The ChangeLog should only contain "what" changed and not "why". So
> just the first part:
>
> Check Py_IsInitialized() before calling PyErr_SetInterrupt().
>
> is sufficient. However, the why should be contained in the commit
> message. You did a good job at explaining the problem in the bug you
> filed, so I think you could just take that and put it in the commit
> log (and massage it a bit if needed).
Also, I think it's worth mentioning that the ChangeLog lines shouldn't
exceed 76 chars (soft limit). And the file 'python.c' is inside the
python/ directory. So a good example would be:
yyyy-mm-dd Name <email>
PR python/21460
* python/python.c: Check Py_IsInitialized() before calling
PyErr_SetInterrupt().
Cheers,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-15 20:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-25 16:26 [PATCH][PR python/21460] Avoid segfault during Python cleanup paul cannon
2017-06-11 20:59 ` Simon Marchi
2017-06-15 20:30 ` Sergio Durigan Junior
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).