From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8226 invoked by alias); 5 Jul 2011 02:26:08 -0000 Received: (qmail 8218 invoked by uid 22791); 5 Jul 2011 02:26:07 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: sourceware.org Received: from mail-gw0-f54.google.com (HELO mail-gw0-f54.google.com) (74.125.83.54) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 05 Jul 2011 02:25:52 +0000 Received: by gwb15 with SMTP id 15so2691826gwb.13 for ; Mon, 04 Jul 2011 19:25:51 -0700 (PDT) MIME-Version: 1.0 Received: by 10.236.182.8 with SMTP id n8mr4809583yhm.237.1309832751399; Mon, 04 Jul 2011 19:25:51 -0700 (PDT) Received: by 10.236.95.167 with HTTP; Mon, 4 Jul 2011 19:25:51 -0700 (PDT) In-Reply-To: References: Date: Tue, 05 Jul 2011 02:47:00 -0000 Message-ID: Subject: Re: [patch] [python] find_line_pc_range From: Matt Rice To: pmuldoon@redhat.com Cc: gdb-patches@sourceware.org Content-Type: multipart/mixed; boundary=20cf30549a19dcc16004a7493333 X-IsSubscribed: yes 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-07/txt/msg00130.txt.bz2 --20cf30549a19dcc16004a7493333 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-length: 3474 On Mon, Jul 4, 2011 at 2:13 AM, Phil Muldoon wrote: > Matt Rice writes: > >> Thanks, attached is an updated patch that also includes tests. > > Thanks. > >> + =A0 =A0 =A0ret_tuple =3D PyTuple_New (2); >> + =A0 =A0 =A0if (!ret_tuple) >> + =A0 =A0 =A0 =A0return Py_None; > > Return NULL here. =A0This signifies an error, and that we have abandoned > the function. =A0When a Python function returns NULL, the exception is > eventually printed and cleared either by the callers, or by the GDB > Python top-level exception handling code. Thanks for explaining, that is what I wanted. You said so in your first em= ail, but for some reason it didn't latch on, sorry. >> + =A0 =A0 =A0start =3D gdb_py_long_from_ulongest (start_pc); >> + =A0 =A0 =A0if (!start) >> + =A0 =A0 goto fail_ret; >> + =A0 =A0 =A0if (PyTuple_SetItem (ret_tuple, 0, start) !=3D 0) >> + =A0 =A0 { >> + =A0 =A0 =A0 Py_XDECREF (start); >> + =A0 =A0 =A0 goto fail_ret; >> + =A0 =A0 } > > You do not need to decrement "start" here. =A0Even if the tuple SetItem > call failed, you have already passed the responsibility to the tuple, > and you no longer own it (stolen reference). =A0SetItem would decrement > this on failure. =A0So just goto to your local error block. OK, the docs were silent on this regard, and I guessed on error it wouldn't steal it. >> + =A0 =A0 =A0end =3D gdb_py_long_from_ulongest (end_pc); >> + =A0 =A0 =A0if (!end) >> + =A0 =A0 goto fail_ret; >> + =A0 =A0 =A0if (PyTuple_SetItem (ret_tuple, 1, end) !=3D 0) >> + =A0 =A0 { >> + =A0 =A0 =A0 Py_XDECREF (end); >> + =A0 =A0 =A0 goto fail_ret; >> + =A0 =A0 } > > Same as above. > >> + =A0 =A0 =A0fail_ret: >> + =A0 =A0 =A0 =A0Py_XDECREF (ret_tuple); > > Small nit, and this is generally ok, but you already know that if this > error block is called, ret_tuple will be instantiated. So in this case > the XDECREF is a tiny bit redundant. =A0You could just use DECREF here and > skip the NULL check that XDECREF does. =A0Like I said, a tiny nit, but I > just thought I would point it out. k. >> + =A0 =A0 return Py_None; > > If there is a Python exception that you cannot handle internally to your > function, you must always return NULL. =A0This informs callers, and > ultimately the exception managing code in GDB that some Python call > failed, to print the exception, and clear it. > >> @@ -526,6 +573,8 @@ static PyMethodDef sal_object_methods[] =3D { >> =A0 =A0{ "is_valid", salpy_is_valid, METH_NOARGS, >> =A0 =A0 =A0"is_valid () -> Boolean.\n\ >> =A0Return true if this symbol table and line is valid, false if not." }, >> + =A0{ "find_line_pc_range", salpy_find_line_pc_range, METH_NOARGS, >> + =A0 =A0"Return a tuple of start and end ranges for the symbol table." = }, >> =A0 =A0{NULL} =A0/* Sentinel */ >> =A0}; > > We try to put the return type in the help now as a formal statement. > Look at the two-line help example above: "is_valid()" for guidance. Updated that text a little bit also, along with using Eli's text for the ma= nual. 2011-07-04 Matt Rice * python/py-symtab.c: Populate sal_object_methods. (salpy_find_line_pc_range): New function. 2011-07-04 Matt Rice Eli Zaretskii * gdb.texinfo (Symbol Tables In Python): Add find_line_pc_range met= hod. 2011-07-04 Matt Rice * gdb.python/py-symtab.exp: New Tests for find_line_pc_range. --20cf30549a19dcc16004a7493333 Content-Type: application/octet-stream; name="foo.diff" Content-Disposition: attachment; filename="foo.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_gpq86sns0 Content-length: 4783 ZGlmZiAtLWdpdCBhL2dkYi9kb2MvZ2RiLnRleGluZm8gYi9nZGIvZG9jL2dk Yi50ZXhpbmZvCmluZGV4IGRiYWYzMGUuLmUxODM2OTMgMTAwNjQ0Ci0tLSBh L2dkYi9kb2MvZ2RiLnRleGluZm8KKysrIGIvZ2RiL2RvYy9nZGIudGV4aW5m bwpAQCAtMjMyOTMsNiArMjMyOTMsMTQgQEAgZXhpc3QgaW4gQHZhbHVle0dE Qk59IGFueSBsb25nZXIuICBBbGwgb3RoZXIKIEBjb2Rle2dkYi5TeW10YWJf YW5kX2xpbmV9IG1ldGhvZHMgd2lsbCB0aHJvdyBhbiBleGNlcHRpb24gaWYg aXQgaXMKIGludmFsaWQgYXQgdGhlIHRpbWUgdGhlIG1ldGhvZCBpcyBjYWxs ZWQuCiBAZW5kIGRlZm1ldGhvZAorCitAZGVmbWV0aG9kIFN5bXRhYl9hbmRf bGluZSBmaW5kX2xpbmVfcGNfcmFuZ2UKK0F0dGVtcHQgdG8gZmluZCB0aGUg cmFuZ2Ugb2YgcHJvZ3JhbSBjb3VudGVyIGFkZHJlc3NlcyBmb3IgdGhlCitA Y29kZXtsaW5lfSBhdHRyaWJ1dGUgb2YgdGhlIEBjb2Rle1N5bXRhYl9hbmRf bGluZX0gb2JqZWN0LiAgSWYKK2ZvdW5kLCByZXR1cm4gYSBAY29kZXtUdXBs ZX0gY29udGFpbmluZyB0aGUgc3RhcnQgYW5kIGVuZCBhZGRyZXNzZXMKK2Zv ciB0aGUgbGluZS4gIE90aGVyd2lzZSAoZS5nLiwgYSBsaW5lIHdpdGggbm8g Y29ycmVzcG9uZGluZyBjb2RlIG9yCitub3QgcHJlc2VudCBpbiB0aGUgZGVi dWcgaW5mbyksIHJldHVybiBAY29kZXtOb25lfS4KK0BlbmQgZGVmbWV0aG9k CiBAZW5kIHRhYmxlCiAKIEEgQGNvZGV7Z2RiLlN5bXRhYn0gb2JqZWN0IGhh cyB0aGUgZm9sbG93aW5nIGF0dHJpYnV0ZXM6CmRpZmYgLS1naXQgYS9nZGIv cHl0aG9uL3B5LXN5bXRhYi5jIGIvZ2RiL3B5dGhvbi9weS1zeW10YWIuYwpp bmRleCAxMDdjZGVjLi41YWUxOTc1IDEwMDY0NAotLS0gYS9nZGIvcHl0aG9u L3B5LXN5bXRhYi5jCisrKyBiL2dkYi9weXRob24vcHktc3ltdGFiLmMKQEAg LTIxNSw2ICsyMTUsNDYgQEAgc2FscHlfZ2V0X2xpbmUgKFB5T2JqZWN0ICpz ZWxmLCB2b2lkICpjbG9zdXJlKQogfQogCiBzdGF0aWMgUHlPYmplY3QgKgor c2FscHlfZmluZF9saW5lX3BjX3JhbmdlIChQeU9iamVjdCAqc2VsZiwgUHlP YmplY3QgKmFyZ3MpCit7CisgIHN0cnVjdCBzeW10YWJfYW5kX2xpbmUgKnNh bCA9IE5VTEw7CisgIENPUkVfQUREUiBzdGFydF9wYywgZW5kX3BjOworCisg IFNBTFBZX1JFUVVJUkVfVkFMSUQgKHNlbGYsIHNhbCk7CisKKyAgaWYgKGZp bmRfbGluZV9wY19yYW5nZSAoKnNhbCwgJnN0YXJ0X3BjLCAmZW5kX3BjKSkK KyAgICB7CisgICAgICBQeU9iamVjdCAqcmV0X3R1cGxlOworICAgICAgUHlP YmplY3QgKnN0YXJ0OworICAgICAgUHlPYmplY3QgKmVuZDsKKworICAgICAg cmV0X3R1cGxlID0gUHlUdXBsZV9OZXcgKDIpOworICAgICAgaWYgKCFyZXRf dHVwbGUpCisgICAgICAgIHJldHVybiBOVUxMOworCisgICAgICBzdGFydCA9 IGdkYl9weV9sb25nX2Zyb21fdWxvbmdlc3QgKHN0YXJ0X3BjKTsKKyAgICAg IGlmICghc3RhcnQpCisJZ290byBmYWlsX3JldDsKKyAgICAgIGlmIChQeVR1 cGxlX1NldEl0ZW0gKHJldF90dXBsZSwgMCwgc3RhcnQpICE9IDApCisJZ290 byBmYWlsX3JldDsKKworICAgICAgZW5kID0gZ2RiX3B5X2xvbmdfZnJvbV91 bG9uZ2VzdCAoZW5kX3BjKTsKKyAgICAgIGlmICghZW5kKQorCWdvdG8gZmFp bF9yZXQ7CisgICAgICBpZiAoUHlUdXBsZV9TZXRJdGVtIChyZXRfdHVwbGUs IDEsIGVuZCkgIT0gMCkKKwlnb3RvIGZhaWxfcmV0OworCisgICAgICByZXR1 cm4gcmV0X3R1cGxlOworCisgICAgICBmYWlsX3JldDoKKwlQeV9ERUNSRUYg KHJldF90dXBsZSk7CisJcmV0dXJuIE5VTEw7CisgICAgfQorCisgIHJldHVy biBQeV9Ob25lOworfQorCitzdGF0aWMgUHlPYmplY3QgKgogc2FscHlfZ2V0 X3N5bXRhYiAoUHlPYmplY3QgKnNlbGYsIHZvaWQgKmNsb3N1cmUpCiB7CiAg IHN0cnVjdCBzeW10YWJfYW5kX2xpbmUgKnNhbDsKQEAgLTUyNiw2ICs1NjYs MTAgQEAgc3RhdGljIFB5TWV0aG9kRGVmIHNhbF9vYmplY3RfbWV0aG9kc1td ID0gewogICB7ICJpc192YWxpZCIsIHNhbHB5X2lzX3ZhbGlkLCBNRVRIX05P QVJHUywKICAgICAiaXNfdmFsaWQgKCkgLT4gQm9vbGVhbi5cblwKIFJldHVy biB0cnVlIGlmIHRoaXMgc3ltYm9sIHRhYmxlIGFuZCBsaW5lIGlzIHZhbGlk LCBmYWxzZSBpZiBub3QuIiB9LAorICB7ICJmaW5kX2xpbmVfcGNfcmFuZ2Ui LCBzYWxweV9maW5kX2xpbmVfcGNfcmFuZ2UsIE1FVEhfTk9BUkdTLAorICAg ICJmaW5kX2xpbmVfcGNfcmFuZ2UgKCkgLT4gVHVwbGUuXG5cCitSZXR1cm4g YSB0dXBsZSBvZiBzdGFydCBhbmQgZW5kIHJhbmdlcyBmb3IgdGhlIHN5bWJv bCB0YWJsZSdzIGxpbmUsXG5cCitvciBOb25lIGlmIG5vdCBmb3VuZC4iIH0s CiAgIHtOVUxMfSAgLyogU2VudGluZWwgKi8KIH07CiAKZGlmZiAtLWdpdCBh L2dkYi90ZXN0c3VpdGUvZ2RiLnB5dGhvbi9weS1zeW10YWIuZXhwIGIvZ2Ri L3Rlc3RzdWl0ZS9nZGIucHl0aG9uL3B5LXN5bXRhYi5leHAKaW5kZXggYzUy ZjVlZi4uZTI3YzA2NiAxMDA2NDQKLS0tIGEvZ2RiL3Rlc3RzdWl0ZS9nZGIu cHl0aG9uL3B5LXN5bXRhYi5leHAKKysrIGIvZ2RiL3Rlc3RzdWl0ZS9nZGIu cHl0aG9uL3B5LXN5bXRhYi5leHAKQEAgLTU4LDYgKzU4LDE0IEBAIGdkYl90 ZXN0ICJweXRob24gcHJpbnQgc2FsLnN5bXRhYiIgImdkYi90ZXN0c3VpdGUv Z2RiLnB5dGhvbi9weS1zeW1ib2wuYy4qIiAiVGVzCiBnZGJfdGVzdCAicHl0 aG9uIHByaW50IHNhbC5wYyIgIiR7ZGVjaW1hbH0iICJUZXN0IHNhbC5wYyIK IGdkYl90ZXN0ICJweXRob24gcHJpbnQgc2FsLmxpbmUiICI0MiIgIlRlc3Qg c2FsLmxpbmUiCiBnZGJfdGVzdCAicHl0aG9uIHByaW50IHNhbC5pc192YWxp ZCgpIiAiVHJ1ZSIgIlRlc3Qgc2FsLmlzX3ZhbGlkIgorZ2RiX3Rlc3QgInB5 dGhvbiBwcmludCBzYWwucGMgPT0gc2FsLmZpbmRfbGluZV9wY19yYW5nZSgp XFswXF0iICJUcnVlIiAiVGVzdCBzYWwuZmluZF9saW5lX3BjX3JhbmdlKCkg MSIKK2dkYl90ZXN0ICJweXRob24gcHJpbnQgc2FsLmZpbmRfbGluZV9wY19y YW5nZSgpXFsxXF0iICIke2RlY2ltYWx9IiAiVGVzdCBzYWwuZmluZF9saW5l X3BjX3JhbmdlKCkgMiIKKworIyBUZXN0IHNhbCBmaW5kX2xpbmVfcGNfcmFu Z2UgZXJyb3JzLgorZ2RiX3Rlc3QgInB5dGhvbiBiYWRfbGluZV9zYWwgPSBn ZGIuZGVjb2RlX2xpbmUoXCJweS1zeW1ib2wuYzo0MDRcIilcWzFcXVxbMFxd IiAiIiBcCisidGVzdCBkZWNvZGVfbGluZSBweS1zeW1ib2wuYzo0MDQiCitn ZGJfdGVzdCAicHl0aG9uIHByaW50IGJhZF9saW5lX3NhbC5maW5kX2xpbmVf cGNfcmFuZ2UoKSA9PSBOb25lIiAiVHJ1ZSIgXAorImZpbmRfbGluZV9wY19y YW5nZSBmb3Igbm9uLWV4aXN0ZW50IGxpbmUiCiAKICMgVGVzdCBzeW1ib2wg dGFibGUuCiBnZGJfdGVzdCAicHl0aG9uIHByaW50IHN5bXRhYi5maWxlbmFt ZSIgInRlc3RzdWl0ZS9nZGIucHl0aG9uL3B5LXN5bWJvbC5jLioiICJUZXN0 IHN5bXRhYi5maWxlbmFtZSIK --20cf30549a19dcc16004a7493333--