From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32414 invoked by alias); 28 Aug 2002 18:36:19 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 32403 invoked from network); 28 Aug 2002 18:36:18 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by sources.redhat.com with SMTP; 28 Aug 2002 18:36:18 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id g7SIV4D17431; Wed, 28 Aug 2002 11:31:04 -0700 X-Authentication-Warning: jackfruit.Stanford.EDU: carlton set sender to carlton@math.stanford.edu using -f To: Daniel Jacobowitz Cc: gdb-patches@sources.redhat.com, ezannoni@redhat.com Subject: Re: RFA: Correct field names for class methods References: <20020827031346.GA16591@nevyn.them.org> <20020828173429.GA5873@nevyn.them.org> Content-Type: text/plain; charset=US-ASCII From: David Carlton Date: Wed, 28 Aug 2002 11:50:00 -0000 In-Reply-To: <20020828173429.GA5873@nevyn.them.org> Message-ID: User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 X-SW-Source: 2002-08/txt/msg00949.txt.bz2 In article <20020828173429.GA5873@nevyn.them.org>, Daniel Jacobowitz writes: > On Wed, Aug 28, 2002 at 10:23:06AM -0700, David Carlton wrote: >> In article <20020827031346.GA16591@nevyn.them.org>, Daniel Jacobowitz >> writes: >> > +void >> > +update_method_name_from_physname (char **old_name, char *physname) >> > +{ >> > + char *method_name; >> > + >> > + method_name = method_name_from_physname (physname); >> > + >> > + if (method_name == NULL) >> > + error ("bad physname %s\n", physname); >> > + >> > + if (strcmp (*old_name, method_name) != 0) >> > + *old_name = method_name; >> > + else >> > + xfree (method_name); >> > +} >> >> I'm pretty sure this is a memory leak. How about replacing the last >> if clause by >> >> xfree(*old_name); >> *old_name = method_name; >> >> That should get rid of the memory leak and avoid a superfluous >> strcmp. > No can do. Look at where TYPE_NAME is allocated; sometimes (often? > Not sure.) it is on the obstack. Eek! I hadn't followed all the code paths. Unfortunate. Having said that, update_method_name_from_physname() is called in exactly one place, with the first argument equal to the location of new_fnlist->fn_fieldlist.name, and at that point in the code, that points to main_fn_name, which is always allocated using xmalloc(). It's only on other code paths that new_fnlist->fn_fieldlist.name gets allocated on an obstack. So I think my fix would be safe with the code as is, but future users of the update_method_name_from_physname() would have to be careful. So it seems that either my fix should be applied with a comment added to the definition of update_method_name_from_physname() saying not to call it unless it's safe to xfree() the first argument, or else it should be left your way, but perhaps with a FIXME comment added. I have no idea which one of those would be better. (Or, now that I think about it, maybe the fix is for main_fn_name to be allocated on an obstack, and for update_method_name_from_physname() also to use an obstack. Hmm.) David Carlton carlton@math.stanford.edu