Re: RFA: Correct field names for class methods

[David Carlton <carlton@math.stanford.edu>]

In article <20020828173429.GA5873@nevyn.them.org>, Daniel Jacobowitz
<drow@mvista.com> writes:
> On Wed, Aug 28, 2002 at 10:23:06AM -0700, David Carlton wrote:
>> In article <20020827031346.GA16591@nevyn.them.org>, Daniel Jacobowitz
>> <drow@mvista.com> 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