Re: RFA: Patch: implement missing macro functions

[Tom Tromey <tromey@redhat.com>]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> You'll need someone else to comment on the big picture -- I'll
Pedro> comment on a few strokes, below,

Thanks for taking the time to do this.

>> +  cleanup_chain = make_my_cleanup (&cleanup_chain, free_current_contents,
>> +                                  &name);

Pedro> That is a big nop to write + with 2 leaks attached.  The new cleanup is
Pedro> being discarded and leaked, so NAME isn't being free'd at all.

Pedro> Don't call make_my_cleanup directly.  Instead, call
Pedro> make_cleanup and discard its result:

Ok, thanks.  I forgot to mention in my note that I wanted someone to
look at this.  I don't actually understand the cleanup mechanism... I
looked a bit but didn't see any documentation on how to use it.

>> +  gdb_flush (gdb_stdout);

Pedro> Did you really need a gdb_flush here?

Probably not.  I'll remove it and see.

>>  
>> +/* Helper function for macro_for_each.  */
>> +static int
>> +foreach_macro (splay_tree_node node, void *fnp)
>> +{
>> +  macro_callback_fn fn = (macro_callback_fn) fnp;

Pedro> (Hmmm, can we assume casting void* <-> func pointers is safe on all
Pedro> supported hosts/compilers?  Posix and Windows do require it to
Pedro> be safe.  I actually have no idea how much GDB common code relies
Pedro> on it today.)

I'll just fix it.

Pedro> Would it make sense to have a test with macro arguments, and
Pedro> a test where a user macro overrides a source macro?

Yeah.  Lazy me!  I will add those.

Tom