Re: [PATCH 2/7] [python] API for macros: memory management quirks.

[Matt Rice <ratmice at gmail dot com>]

On Tue, Aug 30, 2011 at 4:47 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> matt rice <ratmice@gmail.com> 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,
>> ? ?/* We don't create a macro table for this compilation unit
>> ? ? ? at all until we actually get a filename. ?*/
>> ? ?if (! pending_macros)
>> - ? ?pending_macros = new_macro_table (&objfile->objfile_obstack,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?objfile->macro_cache);
>> + ? ?pending_macros = new_macro_table (objfile, macro_table_type_from_objfile);
>
> The comments in macrotab.h need to be updated with the new parameter
> info. ?This 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. ?But what benefit do we accrue now that we purely store
> macros in the objfile's obstack over (the now defunct) bcache? ?Was
> there an underlying reason for dual this you discovered? ?It 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 @@
>>
>> ?struct macro_table
>> ?{
>> - ?/* The obstack this table's data should be allocated in, or zero if
>> - ? ? we should use xmalloc. ?*/
>> - ?struct obstack *obstack;
>> -
>> - ?/* The bcache we should use to hold macro names, argument names, and
>> - ? ? definitions, or zero if we should use xmalloc. ?*/
>> - ?struct bcache *bcache;
>> + ?/* The objfile who's obstack this table's data should be allocated in,
>> + ? ? and bcache we should use to hold macro names, argument
>> + ? ? names, and definitions, or zero if we should use xmalloc. */
>> + ?struct 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->obstack
so that we can safely use register_objfile_data(), for validation.