From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 97350 invoked by alias); 4 Jun 2019 12:36:00 -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 97338 invoked by uid 89); 4 Jun 2019 12:36:00 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.4 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.1 spammy= X-HELO: mga09.intel.com Received: from mga09.intel.com (HELO mga09.intel.com) (134.134.136.24) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Jun 2019 12:35:59 +0000 Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Jun 2019 05:35:57 -0700 Received: from irsmsx102.ger.corp.intel.com ([163.33.3.155]) by FMSMGA003.fm.intel.com with ESMTP; 04 Jun 2019 05:35:56 -0700 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.93]) by IRSMSX102.ger.corp.intel.com ([169.254.2.108]) with mapi id 14.03.0415.000; Tue, 4 Jun 2019 13:35:56 +0100 From: "Metzger, Markus T" To: "Willgerodt, Felix" , "gdb-patches@sourceware.org" Subject: RE: [PATCH 05/10] python: Introduce gdb.RecordAuxiliary class. Date: Tue, 04 Jun 2019 12:36:00 -0000 Message-ID: References: <1559119673-30516-1-git-send-email-felix.willgerodt@intel.com> <1559119673-30516-6-git-send-email-felix.willgerodt@intel.com> In-Reply-To: <1559119673-30516-6-git-send-email-felix.willgerodt@intel.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-06/txt/msg00055.txt.bz2 Hello Felix, > Auxiliary instructions are no real instructions and get their own object > class, similar to gaps. gdb.Record.instruction_history is now possibly a > list of gdb.RecordInstruction, gdb.RecordGap or gdb.RecordAuxiliary > objects. >=20 > This patch is in preparation for the new ptwrite feature, which is based = on > auxiliary instructions. >=20 > 2019-05-29 Felix Willgerodt >=20 > gdb/ChangeLog: > * py-record-btrace.c (btpy_insn_or_gap_new): Removed. > (btpy_item_new): New function. > (btpy_list_item): Call btpy_item_new instead of recpy_insn_new. > (recpy_bt_replay_position): Call btpy_item_new instead of > btpy_insn_or_gap_new. > (recpy_bt_begin): Call btpy_item_new instead of btpy_insn_or_gap_new. > (recpy_bt_end): Call btpy_item_new instead of btpy_insn_or_gap_new. > * py-record.c (recpy_aux_type): New static object. > (recpy_aux_object): New typedef. > (recpy_aux_new, recpy_aux_number, recpy_aux_data): New function. > (recpy_aux_getset): New static object. > (gdbpy_initialize_record): Initialize gdb.RecordAuxiliary type. >=20 > gdb/doc/ChangeLog: > * python.texi (gdb.RecordAuxiliary): New documentation. >=20 > --- > gdb/doc/python.texi | 13 +++++++ > gdb/python/py-record-btrace.c | 22 +++++++---- > gdb/python/py-record.c | 73 ++++++++++++++++++++++++++++++++++- > gdb/python/py-record.h | 3 ++ > 4 files changed, 103 insertions(+), 8 deletions(-) Looks good to me but I can't approve it. > @@ -172,6 +173,14 @@ btpy_insn_or_gap_new (thread_info *tinfo, Py_ssize_t > number) > return recpy_gap_new (err_code, err_string, number); > } >=20 > + const struct btrace_insn *insn =3D btrace_insn_get (&iter); > + > + gdb_assert (insn !=3D nullptr); This must be true because we checked btrace_insn_get_error () before. To m= ake it more obvious, we could rewrite the function to call btrace_insn_get () firs= t and check the returned btrace_insn pointer. There's no need to check the error code,= I think. In the worst case, we'll end up with a gap that says 'success'. > @@ -470,7 +479,7 @@ btpy_list_item (PyObject *self, Py_ssize_t index) > number =3D obj->first + (obj->step * index); >=20 > if (obj->element_type =3D=3D &recpy_insn_type) > - return recpy_insn_new (obj->thread, RECORD_METHOD_BTRACE, number); > + return btpy_item_new (obj->thread, number); > else > return recpy_func_new (obj->thread, RECORD_METHOD_BTRACE, number); > } Isn't this code just creating list item objects? Thanks, Markus. Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928