From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106875 invoked by alias); 11 Jun 2017 20:59:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 106864 invoked by uid 89); 11 Jun 2017 20:59:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=HTo:U*paul, massage, HContent-Transfer-Encoding:8bit X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 11 Jun 2017 20:59:16 +0000 Received: by simark.ca (Postfix, from userid 33) id 6D5351E4D7; Sun, 11 Jun 2017 16:59:19 -0400 (EDT) To: paul cannon Subject: Re: [PATCH][PR python/21460] Avoid segfault during Python cleanup X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Sun, 11 Jun 2017 20:59:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <20170525162612.GA10119@turing> References: <20170525162612.GA10119@turing> Message-ID: <69e1e2d6b7054702d39a081238590633@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00264.txt.bz2 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   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