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

["Simon Marchi (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

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