From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10998 invoked by alias); 1 Sep 2011 21:48:39 -0000 Received: (qmail 10989 invoked by uid 22791); 1 Sep 2011 21:48:38 -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,TW_BJ,T_TO_NO_BRKTS_FREEMAIL X-Spam-Check-By: sourceware.org Received: from mail-gx0-f169.google.com (HELO mail-gx0-f169.google.com) (209.85.161.169) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 01 Sep 2011 21:48:23 +0000 Received: by gxk23 with SMTP id 23so1863462gxk.0 for ; Thu, 01 Sep 2011 14:48:22 -0700 (PDT) MIME-Version: 1.0 Received: by 10.236.187.104 with SMTP id x68mr1886845yhm.130.1314913702767; Thu, 01 Sep 2011 14:48:22 -0700 (PDT) Received: by 10.236.34.162 with HTTP; Thu, 1 Sep 2011 14:48:22 -0700 (PDT) In-Reply-To: References: <1314198654-9008-1-git-send-email-ratmice@gmail.com> <1314198654-9008-3-git-send-email-ratmice@gmail.com> Date: Thu, 01 Sep 2011 22:46:00 -0000 Message-ID: Subject: Re: [PATCH 2/7] [python] API for macros: memory management quirks. From: Matt Rice To: pmuldoon@redhat.com Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable 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-09/txt/msg00019.txt.bz2 On Tue, Aug 30, 2011 at 4:47 AM, Phil Muldoon wrote: > matt rice writes: > >> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c >> index f66c1d9..c56f431 100644 >> --- a/gdb/dwarf2read.c >> +++ b/gdb/dwarf2read.c >> @@ -14351,8 +14351,7 @@ macro_start_file (int file, int line, >> =A0 =A0/* We don't create a macro table for this compilation unit >> =A0 =A0 =A0 at all until we actually get a filename. =A0*/ >> =A0 =A0if (! pending_macros) >> - =A0 =A0pending_macros =3D new_macro_table (&objfile->objfile_obstack, >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0objfile->macro_cache); >> + =A0 =A0pending_macros =3D new_macro_table (objfile, macro_table_type_f= rom_objfile); > > The comments in macrotab.h need to be updated with the new parameter > info. =A0This and others. > > This is code I am not very sure of, having never dabbled in this area. > So this review would benefit from a maintainer that is more familiar > with this code. =A0But what benefit do we accrue now that we purely store > macros in the objfile's obstack over (the now defunct) bcache? =A0Was > there an underlying reason for dual this you discovered? =A0It seems odd = that > in the pre-patched version that struct macro_table had two strategies > for this. Contextually, why did you elect to use the obstack in the > objfile, instead of the previous ancillary related structure? forgot reply-to all, Phil (you'll receive 2). I didn't actually change this except to remove the pointers in macro_table, we still use both objfile->obstack and objfile->macro_cache. >> diff --git a/gdb/macrotab.c b/gdb/macrotab.c >> index efcf835..b2825d2 100644 >> --- a/gdb/macrotab.c >> +++ b/gdb/macrotab.c >> @@ -35,13 +35,10 @@ >> >> =A0struct macro_table >> =A0{ >> - =A0/* The obstack this table's data should be allocated in, or zero if >> - =A0 =A0 we should use xmalloc. =A0*/ >> - =A0struct obstack *obstack; >> - >> - =A0/* The bcache we should use to hold macro names, argument names, and >> - =A0 =A0 definitions, or zero if we should use xmalloc. =A0*/ >> - =A0struct bcache *bcache; >> + =A0/* The objfile who's obstack this table's data should be allocated = in, >> + =A0 =A0 and bcache we should use to hold macro names, argument >> + =A0 =A0 names, and definitions, or zero if we should use xmalloc. */ >> + =A0struct objfile *objfile; > > > Comment seems wrong, as bcache data structure no longer exists. > Additionally, (not in the code, but in your email), a narrative why we > are removing the bcache is needed. It could definitely be clearer. e.g. The objfile who's obstack and bcache. i'll make it more clear that this is OBJFILE's bcache in the rest of these comments. as per why: we are replacing bcache and obstack with objfile->bcache, and objfile->obst= ack so that we can safely use register_objfile_data(), for validation.