[review v11] gdb/mi: Add -max-results parameter to some -symbol-info-* commands

Simon Marchi (Code Review)
2019-12-03T04:12:06Z

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/269
......................................................................

Patch Set 11:

(2 comments)

I'm getting a bunch of compilation errors that I am surprised you don't get:

/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c: In function ‘size_t parse_max_results_option(const char*)’:
/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c:177:15: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
177 | char *ptr = arg;
| ^~~
| |
| const char*
/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c: In function ‘void mi_info_functions_or_variables(search_domain, char**, int)’:
/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c:233:29: error: assignment of function ‘size_t parse_max_results_option(const char*)’
233 | parse_max_results_option = parse_max_results_option (oarg);
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c: In function ‘void mi_cmd_symbol_info_modules(const char*, char**, int)’:
/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c:436:29: error: assignment of function ‘size_t parse_max_results_option(const char*)’
436 | parse_max_results_option = parse_max_results_option (oarg);
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c: In function ‘void mi_cmd_symbol_info_types(const char*, char**, int)’:
/home/simark/src/binutils-gdb/gdb/mi/mi-symbol-cmds.c:478:29: error: assignment of function ‘size_t parse_max_results_option(const char*)’
478 | parse_max_results_option = parse_max_results_option (oarg);
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

| --- /dev/null
| +++ /COMMIT_MSG
| @@ -1,0 +17,19 @@ these commands is slightly harder.
| +There's currently no mechanism for the user of these commands to know
| +if the result list has been truncated if you get back the maximum
| +number of results, so if there are exactly 10 functions and you call
| +'-symbol-info-functions --max-results 10' the reply would appear no
| +different than if you had 20 functions and called with a max of 10.
| +Right now, if you get back the maximum then you should assume that
| +there might be more results available.
| +
| +One other thing to note is that the global_symbol_searcher::search by
| +default returns UINT_MAX results, there's no longer a mechanism to

PS11, Line 26:

Since you changed it so use size_t/SIZE_MAX, this should be updated.

| +return an unlimited number of results, though hopefully this will not
| +be a huge issue.
| +
| +gdb/ChangeLog:
| +
| + * mi/mi-symbol-cmds.c (mi_symbol_info): Take extra parameter, and
| + add it into the search spec.
| + (parse_max_results_option): New function.
| + (mi_info_functions_or_variables): Parse -max-results flag and pass
| --- gdb/symtab.c
| +++ gdb/symtab.c
| @@ -4737,9 +4735,19 @@ global_symbol_searcher::search () const
| + RESULT_SET set. Use a set here so that we can easily detect
| + duplicates as we go, and can therefore track how many unique
| + matches we have found so far. */
| + if (!add_matching_symbols (objfile, preg, treg, &result_set))
| + break;
| + }
| +
| + /* Convert the result set into a sorted result list. */
| + std::vector<symbol_search> result (result_set.begin (), result_set.end ());
| + std::sort (result.begin (), result.end ());

PS11, Line 4744:

Please check to be sure, but I think this would be unnecessary, as an
std::set is already sorted. Since you create both the set and the
vector of symbol_search without any explicit comparator, I suppose
they will be sorted using the same criterion.

|
| /* If there are no debug symbols, then add matching minsyms. But if the
| user wants to see symbols matching a type regexp, then never give a
| minimal symbol, as we assume that a minimal symbol does not have a
| type. */
| if ((found_msymbol || (filenames.empty () && m_kind == VARIABLES_DOMAIN))
| && !m_exclude_minsyms
| && !treg.has_value ())
| {

--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I90a28feb55b388fb46461a096c5db08b6b0bd427
Gerrit-Change-Number: 269
Gerrit-PatchSet: 11
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-CC: Joel Brobecker <brobecker@adacore.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 03 Dec 2019 04:11:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment