This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[review] gdb: Introduce symbol_search_spec
- From: "Tom Tromey (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>
- To: Andrew Burgess <andrew dot burgess at embecosm dot com>, gdb-patches at sourceware dot org
- Date: Wed, 30 Oct 2019 10:19:19 -0400
- Subject: [review] gdb: Introduce symbol_search_spec
- Auto-submitted: auto-generated
- References: <gerrit.1571909344000.I488ab292a892d9e9e84775c632c5f198b6ad3710@gnutoolchain-gerrit.osci.io>
- Reply-to: gnutoolchain-gerrit at osci dot io
Tom Tromey has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
Thanks, this looks good. I did have one question about whether search_symbols
should remain a function or should be a method of the new struct.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.h
File gdb/symtab.h:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.h@2094
PS1, Line 2094:
2069 | struct search_symbols_spec
| ...
2093 | /* Constructor. */
2094 | search_symbols_spec (enum search_domain kind,
2095 | const char *symbol_regexp = nullptr,
2096 | const char *type_regexp = nullptr,
2097 | bool exclude_minsyms = false)
2098 | : kind (kind),
2099 | symbol_regexp (symbol_regexp),
2100 | type_regexp (type_regexp),
2101 | exclude_minsyms (exclude_minsyms)
2102 | { /* Nothing. */ }
This could gdb_assert (kind != ALL_DOMAIN)
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c
File gdb/symtab.c:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/264/1/gdb/symtab.c@4449
PS1, Line 4449:
4447 |
4448 | std::vector<symbol_search>
4449 | search_symbols (const search_symbols_spec &search_spec)
4450 | {
4451 | /* Unpack the search spec. */
4452 | const char *regexp = search_spec.symbol_regexp;
4453 | enum search_domain kind = search_spec.kind;
4454 | const char *t_regexp = search_spec.type_regexp;
4455 | int nfiles = search_spec.filenames.size ();
4456 | bool exclude_minsyms = search_spec.exclude_minsyms;
Did you consider making search_symbols a method of
search_symbols_spec? Then this unpacking wouldn't be
needed.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I488ab292a892d9e9e84775c632c5f198b6ad3710
Gerrit-Change-Number: 264
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Burgess <andrew.burgess@embecosm.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Wed, 30 Oct 2019 14:19:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment