From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25128 invoked by alias); 3 Dec 2019 04:12:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 25111 invoked by uid 89); 3 Dec 2019 04:12:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-13.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 03 Dec 2019 04:11:59 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id 0B05620172; Mon, 2 Dec 2019 23:11:57 -0500 (EST) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [8.43.85.239]) by mx1.osci.io (Postfix) with ESMTP id E91B22032B; Mon, 2 Dec 2019 23:11:54 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id D1C9E2816F; Mon, 2 Dec 2019 23:11:54 -0500 (EST) X-Gerrit-PatchSet: 11 Date: Tue, 03 Dec 2019 04:12:00 -0000 From: "Simon Marchi (Code Review)" To: Andrew Burgess , gdb-patches@sourceware.org Cc: Joel Brobecker Auto-Submitted: auto-generated X-Gerrit-MessageType: comment Subject: [review v11] gdb/mi: Add -max-results parameter to some -symbol-info-* commands X-Gerrit-Change-Id: I90a28feb55b388fb46461a096c5db08b6b0bd427 X-Gerrit-Change-Number: 269 X-Gerrit-ChangeURL: X-Gerrit-Commit: d459113fb6f9cad1ef850a166042d1e795334698 In-Reply-To: References: X-Gerrit-Comment-Date: Mon, 2 Dec 2019 23:11:54 -0500 Reply-To: gnutoolchain-gerrit@osci.io MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/3.0.3-79-g83ff7f88f1 Content-Type: text/plain; charset=UTF-8 Message-Id: <20191203041154.D1C9E2816F@gnutoolchain-gerrit.osci.io> X-SW-Source: 2019-12/txt/msg00077.txt.bz2 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 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 Gerrit-Reviewer: Andrew Burgess Gerrit-CC: Joel Brobecker Gerrit-CC: Simon Marchi Gerrit-Comment-Date: Tue, 03 Dec 2019 04:11:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment