From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29636 invoked by alias); 7 Nov 2013 20:10:16 -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 29623 invoked by uid 89); 7 Nov 2013 20:10:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.8 required=5.0 tests=AWL,BAYES_50,RDNS_NONE,SPAM_SUBJECT,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mail-ob0-f201.google.com Received: from Unknown (HELO mail-ob0-f201.google.com) (209.85.214.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 07 Nov 2013 20:08:54 +0000 Received: by mail-ob0-f201.google.com with SMTP id vb8so120929obc.0 for ; Thu, 07 Nov 2013 12:08:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=X5F7QHIgWHflw+ETw67Xl4i8kD4hLFJrOeWU8NfoHfM=; b=moOUO+4I4LwgpOuNbIguDH8hLQHPEFPbJYnsxcF56Y7warW7KR5g+HtuYKf6wLEu1g lZyZZgoYEdRmKrk1WNOBD7aie2Dym5bY9NqWzpb+MD6nWr/91uefDeYWGlPZ3ipRKro8 agZeCP31a6JQwFhnxgo+iKpCLzf8RAazXQleIm9V9xB7bhz8o26BEqrkEG3KBDYqzCSN ME6csWvsranFv8xRjSMYT/3jQknurWglQMsy1WPxPIeXhgEHXLzjL21aiUlCcITfDP3v GUu9mzDePphuZsvZXv4AaEW0igaOKyYMzUDs+Ym6QzCPp6386yFKk7JNxSVI1RVCGkSY 8sSA== X-Gm-Message-State: ALoCoQmUbBCrlJBwTaW8gGSQLX9vAThrhNKVDfQuTS2EQtgxr8fICu2EmEy+ssKOEH+QHA8Lotpi4fiPIMzU45E/SHCRchJGfSEQSYZTpKFBcaF3jjHgADu4iyJMwPGfS9M4kwmMs2AlUZmuc32kglcOBIsXqLf4z0wZlxrc5no6uXtXUZo+6Ll39MiTl1OdnZZpOt+eUA9bZ9nZS47dxxv0NYIjQQEILA== X-Received: by 10.50.164.193 with SMTP id ys1mr1726728igb.6.1383854926746; Thu, 07 Nov 2013 12:08:46 -0800 (PST) Received: from corp2gmr1-2.hot.corp.google.com (corp2gmr1-2.hot.corp.google.com [172.24.189.93]) by gmr-mx.google.com with ESMTPS id k45si732264yhn.4.2013.11.07.12.08.46 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 07 Nov 2013 12:08:46 -0800 (PST) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id 049BA5A41EA; Thu, 7 Nov 2013 12:08:45 -0800 (PST) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21115.62285.436813.473271@ruffy.mtv.corp.google.com> Date: Thu, 07 Nov 2013 20:42:00 -0000 To: Phil Muldoon Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [patch][python] 1/3 Python representation of GDB line tables (Python code) In-Reply-To: <527BC0F3.1080800@redhat.com> References: <5253F4D4.1050600@redhat.com> <87ob6fd8c7.fsf@fleche.redhat.com> <527BC0F3.1080800@redhat.com> X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg00207.txt.bz2 Phil Muldoon writes: > On 23/10/13 21:46, Tom Tromey wrote: > >>>>>> "Phil" == Phil Muldoon writes: > > > > Phil> This patch series (three emails) introduces GDB line table representation > > Phil> to Python. > > > > Phil> This first email contains the GDB and Python code to represent the line > > Phil> table data as Python objects. > > > > Phil> What do you think? > > > > Thanks, Phil. > > > > Really just nits here. > > Fixed up nits. > > > Phil> + /* Return a frozen set to remove duplicated line numbers in the case > > Phil> + where a source line has more than one instruction and more than one > > Phil> + entry in the line table. */ > > Phil> + fset = PyFrozenSet_New (source_list); > > Phil> + Py_DECREF (source_list); > > > > PyFrozenSet_New is new in 2.5. > > > > I think it's cheaper, and no more difficult, to just build a set object > > from the start rather than make a list and the convert it. I'm not sure > > what to about Python 2.4 though. > > All API access to sets is new in 2.5 (sets appeared in the Python > default library in 2.3) > > I elected here just to use a dictionary to ensure unique line numbers > only. > > Modified patch follows, > > Cheers, > > Phil Hi. I have some nits too. Sorry! Fortunately they're all just nits. > diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c > new file mode 100644 > index 0000000..9cde481 > --- /dev/null > +++ b/gdb/python/py-linetable.c > @@ -0,0 +1,621 @@ > +/* Python interface to line tables. > + > + Copyright (C) 2013 Free Software Foundation, Inc. > + > + This file is part of GDB. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +#include "defs.h" > +#include "python-internal.h" > +#include "exceptions.h" > + > +typedef struct { > + PyObject_HEAD > + /* The line table source line. */ > + gdb_py_longest line; Can this be an int? symtab_and_line and linetable_entry use int, thus I'm guessing most/all of the API in gdb proper uses int. Using different types looks weird and I'm left with asking "Why?" I think int will be enough - if not other places will notice sooner than the Python API will. :-) > + /* The pc associated with the source line. */ > + CORE_ADDR pc; > +} linetable_entry_object; > [...] > + > +/* Internal helper function to build a Python Tuple from a GDB Vector. > + A line table entry can have multiple PCs for a given source line. > + Construct a Tuple of all entries for the given source line, LINE > + from the line table VEC. Construct one line table entry object per > + address. */ > + > +static PyObject * > +build_tuple_from_vector (gdb_py_longest line, VEC (CORE_ADDR) *vec) Can this function be renamed? I read this at the call site below and wondered how a general purpose vector-to-tuple routine was the right solution. Then I scolled back up and saw this isn't a general purpose routine, rather it takes a line number and vector of addresses and returns a tuple of line table entries. build_line_table_tuple_from_pcs or whatever works for me, just something that makes it more explicit what it's doing. > +/* Implementation of gdb.LineTable.has_pcs (self, line) -> Boolean. > + Returns a Python Boolean indicating whether a source line has any > + line table entries corresponding to it. */ > + > +static PyObject * > +ltpy_has_pcs (PyObject *self, PyObject *args) Can this function be renamed too? I read "has_pcs" and think it takes a list of pcs (plural) as an argument. I now understand how the name is intended to be read here, but it took a bit. Since the inputs are a line table and a line number, and the result is a boolean indicating if the line number is in the table --> ltpy_has_line or some such? > +/* Implementation of gdb.LineTableEntry.pc (self) -> Unsigned Long. > + Returns an unsigned long associated with the PC of the line table > + entry. */ > + > +static PyObject * > +ltpy_entry_get_pcs (PyObject *self, void *closure) Sorry again. Can this function be renamed too? I read "get_pcs" and think plural. ltpy_entry_get_pc ?