From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6314 invoked by alias); 24 Sep 2009 01:52:53 -0000 Mailing-List: contact archer-help@sourceware.org; run by ezmlm Sender: Precedence: bulk List-Post: List-Help: List-Subscribe: List-Id: Received: (qmail 6303 invoked by uid 22791); 24 Sep 2009 01:52:52 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Message-ID: <4ABAD0EA.4020303@googlemail.com> Date: Thu, 24 Sep 2009 01:52:00 -0000 From: Richard Ward User-Agent: Thunderbird 2.0.0.23 (X11/20090817) MIME-Version: 1.0 To: Oguz Kayral CC: archer@sourceware.org Subject: Re: [RFC][0/5] Python event handling References: <36a35d480908230834y46a44e08od4645db444bb5e7a@mail.gmail.com> In-Reply-To: <36a35d480908230834y46a44e08od4645db444bb5e7a@mail.gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2009-q3/txt/msg00248.txt.bz2 Oguz Kayral wrote: > Hi, > This patch series adds inferior event handling support to GDB Python > scripting. Hi Oguz, I've been looking forward to this functionality, Good stuff! I have a few comments though, if you don't mind. Firstly two (fairly unimportant) things about the function registration. I think it would be nice if you could give upvalues to the registration function (similar to gobject's `connect'), though admittedly it is very easy to fake that in python using a class that defines __call__. Also, I'm not sure whether this would ever be an issue but it seems advantageous to have the registration function return a unique object or number that could subsequently be used to unregister your callable, rather than using the callable object itself. Next, in python-stopevent.c: emit_stop_event (struct bpstats *bs, const char *stop_signal) { /*...snip...*/ for (i = 0; i < PyList_Size (callback_list); i++) { PyObject_CallObject (PyList_GET_ITEM (callback_list, i), args_tuple); } } A couple of issues with this. First, PyObject_Callback returns a new reference which is the return value of the function (or NULL), so it must be handled otherwise it will leak. Secondly the user code may throw an exception, or the object may not be callable. The exception should be cleared, and it would be nice to also print the exception (PyErr_Print does both). Most important here though is the iteration through the list. If the user disconnects a callback from inside a callback then the some callbacks could be skipped. It could be better to copy the list first, but that could still lead to some confusing behaviour, for example a callback being called after the user thinks it ought to have been disconnected. Thanks, Richard