This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]