Re: [PATCH 1/7] [python] API for macros: abort or continuing macro iterators.

[Matt Rice <ratmice@gmail.com>]

On Tue, Aug 30, 2011 at 4:09 AM, Phil Muldoon <pmuldoon@redhat.com> wrote:
> matt rice <ratmice@gmail.com> writes:
>
>> diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
>> index d1ac7fa..e929fe0 100644
>> --- a/gdb/macrocmd.c
>> +++ b/gdb/macrocmd.c
>> @@ -220,13 +220,15 @@ info_macro_command (char *name, int from_tty)
>>     Otherwise USER_DATA is considered to be a string, printing
>>     only macros who's NAME matches USER_DATA.  Other arguments are
>>     routed to print_macro_definition.  */
>> -static void
>> +static enum macro_walk_status
>>  print_macro_callback (const char *name, const struct macro_definition *macro,
>>                  struct macro_source_file *source, int line,
>>                  void *user_data)
>>  {
>>    if (! user_data || strcmp (user_data, name) == 0)
>>      print_macro_definition (name, macro, source, line);
>> +
>> +  return macro_walk_continue;
>>  }
>
> Is the unconditional return here due to it being part of a recursive
> call chain? I'm not sure what value the unconditional return of an enum
> constant over the previous "void" gets us? Why return anything at all?
> The previous version assumed the callers knew how to deal with the void
> return, but I am not sure what your version achieves by unconditionally
> returning macro_walk_continue?

This is just matching pre-existing behavior from before macro
iteration could abort/continue.
the 'void' return was hard coded as 'return 0', in foreach_macro and
foreach_macro_in_scope
where now they return 'status == macro_walk_abort'

where 0 means continue and != 0 abort as interpreted by
src/libiberty/splay-tree.c:splay_tree_foreach_helper

>>  /* Implementation of the "info definitions" command. */
>> @@ -435,7 +437,7 @@ macro_undef_command (char *exp, int from_tty)
>>  }
>>
>>
>> -static void
>> +static enum macro_walk_status
>>  print_one_macro (const char *name, const struct macro_definition *macro,
>>                struct macro_source_file *source, int line,
>>                void *ignore)
>> @@ -452,6 +454,7 @@ print_one_macro (const char *name, const struct macro_definition *macro,
>>        fprintf_filtered (gdb_stdout, ")");
>>      }
>>    fprintf_filtered (gdb_stdout, " %s\n", macro->replacement);
>> +  return macro_walk_continue;
>>  }
>
> Same as above. If you decide to keep them please update the comment
> headers with a description of the return value.  This function,  and
> numerous other places too.

will do, though, these are documented in macrotab.h:macro_callback_fn,
i should at least
mention they are implementations of those, and arguments/return values
are documented there.
does that sound alright?