From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11923 invoked by alias); 24 Oct 2011 15:31:39 -0000 Received: (qmail 11905 invoked by uid 22791); 24 Oct 2011 15:31:36 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from qmta08.westchester.pa.mail.comcast.net (HELO qmta08.westchester.pa.mail.comcast.net) (76.96.62.80) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 24 Oct 2011 15:31:22 +0000 Received: from omta22.westchester.pa.mail.comcast.net ([76.96.62.73]) by qmta08.westchester.pa.mail.comcast.net with comcast id od9n1h00A1ap0As58fXN5C; Mon, 24 Oct 2011 15:31:22 +0000 Received: from [10.127.238.91] ([65.206.2.68]) by omta22.westchester.pa.mail.comcast.net with comcast id ofX91h01U1U2a2h3ifXB7k; Mon, 24 Oct 2011 15:31:20 +0000 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Apple Message framework v1084) Subject: Re: [python] [patch] PR python/13331 From: Paul Koning In-Reply-To: <09787EF419216C41A903FD14EE5506DD030CE5E15C@AUSX7MCPC103.AMER.DELL.COM> Date: Mon, 24 Oct 2011 15:39:00 -0000 Content-Transfer-Encoding: quoted-printable Message-Id: <966B9848-BA1E-4777-BCE3-FD57AEA6E989@comcast.net> References: <09787EF419216C41A903FD14EE5506DD030CE5E15C@AUSX7MCPC103.AMER.DELL.COM> To: gdb-patches@sourceware.org, Tom Tromey , Phil Muldoon 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 X-SW-Source: 2011-10/txt/msg00638.txt.bz2 > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourcewa= re.org] On Behalf Of Tom Tromey > Subject: Re: [python] [patch] PR python/13331 >=20 >>>>>> "Phil" =3D=3D Phil Muldoon writes: >=20 > Phil> 2011-10-24 Phil Muldoon > Phil> PR python/13331 > Phil> * python/py-function.c (fnpy_call): Check 'args' is not NULL. > Phil> (convert_values_to_python): Return on Python tuple allocation > Phil> failure. >=20 > Phil> + if (! result) > Phil> + return NULL; >=20 > I think the function should have a single return convention. > One easy way to do that would be to remove the 'error' call and have it r= eturn NULL when value_to_value_object fails. I think the patch is not quite right because it goes against the Python rul= e for exception handling. A NULL function result is the normal way to indi= cate an error, but associated with returning NULL is that in the spot where= the error is first detected you set the exception status. And in addition= , anywhere else NULL is propagated (with any necessary cleanup) but the exc= eption status is *not* overwritten. > ... > Index: python/py-function.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/python/py-function.c,v > retrieving revision 1.10 > diff -u -r1.10 py-function.c > --- python/py-function.c 5 Aug 2011 14:24:10 -0000 1.10 > +++ python/py-function.c 24 Oct 2011 13:15:38 -0000 > @@ -38,6 +38,9 @@ > { > int i; > PyObject *result =3D PyTuple_New (argc); > +=20=20 > + if (! result) > + return NULL; >=20 > for (i =3D 0; i < argc; ++i) > { In this loop there is an example of what I mentioned: if value_to_value_obj= ect returns NULL, exception status has been set, and that should not be ove= rwritten here. On the other hand, there is missing cleanup: any objects ad= ded to the tuple before the failing call need to have a DECREF done on them. > @@ -59,24 +62,35 @@ > void *cookie, int argc, struct value **argv) { > struct value *value =3D NULL; > - PyObject *result, *callable, *args; > + /* 'result' must be set to NULL, this initially indicates whether > + the function was called, or not. */ PyObject *result =3D NULL;=20= =20 > + PyObject *callable, *args; > struct cleanup *cleanup; >=20 > cleanup =3D ensure_python_env (gdbarch, language); >=20 > args =3D convert_values_to_python (argc, argv); > + /* convert_values_to_python can return NULL on error. If we > + encounter this, do not call the function, but allow the Python -> > + error code conversion below to deal with the Python exception. > + Note, that this is different if the function simply does not > + have arguments. */ >=20 > - callable =3D PyObject_GetAttrString ((PyObject *) cookie, "invoke"); > - if (! callable) > + if (args) > { > + callable =3D PyObject_GetAttrString ((PyObject *) cookie, "invoke"= ); > + if (! callable) > + { > + Py_DECREF (args); > + error (_("No method named 'invoke' in object.")); That should just be a cleanup and return NULL -- the PyObject_GetAttrString= has set the exception. paul