Re: PATCH: add support for --dynamic-list and friends

[Ian Lance Taylor <iant at google dot com>]

csilvers@google.com (Craig Silverstein) writes:

> 2008-11-05  Craig Silverstein  <csilvers@google.com>
>
> 	* options.cc (General_options::parse_dynamic_list): New function.
> 	* options.h (General_options): New flags dynamic_list,
> 	dynamic_list_data, dynamic_list_cpp_new, and
> 	dynamic_list_cpp_typeinfo.  New variable dynamic_list_.
> 	(General_options::in_dynamic_list): New function.
> 	* script.cc (Lex::Mode): New enum DYNAMIC_LIST.
> 	(Lex::can_start_name): Add support for DYNAMIC_LIST mode.
> 	(Lex::can_continue_name): Likewise.
> 	(yylex): Likewise.
> 	(read_script_file): New parameter script_options.
> 	(read_dynamic_list): New function.
> 	(Script_options::define_dynamic_list): New function.
> 	(dynamic_list_keyword_parsecodes): New variable.
> 	(dynamic_list_keywords): New variable.
> 	* script.h (Script_options::define_dynamic_list): New function
> 	prototype.
> 	(read_dynamic_list): New function prototype.
> 	* symtab.cc (strprefix): New macro.
> 	(Symbol::should_add_dynsym_entry): Support dynamic_list,
> 	dynamic_list_data, dynamic_list_cpp_new, and
> 	dynamic_list_cpp_typeinfo.
> 	* yyscript.y (PARSING_DYNAMIC_LIST): New token.
> 	(dynamic_list_expr): New rule.
> 	(dynamic_list_nodes): Likewise.
> 	(dynamic_list_node): Likewise.
> 	* testsuite/Makefile.am (dynamic_list): New test.
> 	* testsuite/Makefile.in: Regenerated.
> 	* testsuite/dynamic_list.t: New file.
> 	* testsuite/dynamic_list.sh: New file.
> 	* testsuite/dynamic_list.stdout: New file.



> +// FILENAME was found as an argument to --dynamic-list.  Read it as a
> +// list of symbols, and store its contents in
> +// cmdline->script_options()->dynamic_list().
> +
> +bool
> +read_dynamic_list(const char* filename, Command_line* cmdline,
> +                  Script_options* dynamic_list)
> +{
> +  return read_script_file(filename, cmdline, dynamic_list,
> +                          PARSING_DYNAMIC_LIST, Lex::DYNAMIC_LIST);
> +}

I think the comment is wrong--I think it stores the contents in
DYNAMIC_LIST.


> +// FILE was found as an argument to --dynamic-list.  Read it as a
> +// version script (actually, a versym_node from a version script), and
> +// store its contents in cmdline->dynamic_list().
> +
> +bool
> +read_dynamic_list(const char* filename, Command_line* cmdline,
> +                  Script_options* dynamic_list);
> +

Comment should refer to FILENAME, not FILE.  And like the above, I
think it stores the contents in DYNAMIC_LIST.


> +// The ""'s around str ensure str is a string literal, so sizeof works.
> +#define strprefix(var, str)   (strncmp(var, str, sizeof("" str "")-1) == 0)

Please put spaces around the '-'--this may require breaking the line.


> --- /dev/null	2007-10-18 09:27:25.000000000 -0700
> +++ testsuite/dynamic_list.stdout	2008-11-05 00:01:38.980320000 -0800

You don't really want this file to the repository, right?


Looks very good.  Please make the above change and commit.

Thanks.

Ian