From patchwork Mon Sep 11 00:33:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 22815 Received: (qmail 97490 invoked by alias); 11 Sep 2017 00:33:40 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 97423 invoked by uid 89); 11 Sep 2017 00:33:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: gproxy3-pub.mail.unifiedlayer.com Received: from gproxy3-pub.mail.unifiedlayer.com (HELO gproxy3-pub.mail.unifiedlayer.com) (69.89.30.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 11 Sep 2017 00:33:36 +0000 Received: from cmgw3 (unknown [10.0.90.84]) by gproxy3.mail.unifiedlayer.com (Postfix) with ESMTP id 39425401CF for ; Sun, 10 Sep 2017 18:33:35 -0600 (MDT) Received: from box522.bluehost.com ([74.220.219.122]) by cmgw3 with id 80ZY1w00J2f2jeq010Zbps; Sun, 10 Sep 2017 18:33:35 -0600 X-Authority-Analysis: v=2.2 cv=K/VSJ2eI c=1 sm=1 tr=0 a=GsOEXm/OWkKvwdLVJsfwcA==:117 a=GsOEXm/OWkKvwdLVJsfwcA==:17 a=2JCJgTwv5E4A:10 a=zstS-IiYAAAA:8 a=QyXUC8HyAAAA:8 a=Ivp-YTrx31e0W54YIJoA:9 a=Oa4jj1KKRT8vPK0J:21 a=1bhFNMK4pTWL9MRd:21 a=4G6NA9xxw8l3yy4pmD5M:22 Received: from 75-166-76-94.hlrn.qwest.net ([75.166.76.94]:39766 helo=bapiya.Home) by box522.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.87) (envelope-from ) id 1drCfL-003b1Y-TV; Sun, 10 Sep 2017 18:33:32 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA 3/3] Make extract_arg return a std::string Date: Sun, 10 Sep 2017 18:33:25 -0600 Message-Id: <20170911003325.3765-4-tom@tromey.com> In-Reply-To: <20170911003325.3765-1-tom@tromey.com> References: <20170911003325.3765-1-tom@tromey.com> X-BWhitelist: no X-Exim-ID: 1drCfL-003b1Y-TV X-Source-Sender: 75-166-76-94.hlrn.qwest.net (bapiya.Home) [75.166.76.94]:39766 X-Source-Auth: tom+tromey.com X-Email-Count: 4 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTIyLmJsdWVob3N0LmNvbQ== X-Local-Domain: yes Change extract_arg to return a std::string and fix up all the users. I think string is mildly better than unique_xmalloc_ptr, when possible, because it provides a more robust API. I changed the error messages emitted from find_location_by_number to avoid either writing to a string or an extra allocation; this can be changed but I thought that the new message was not any less clear. You can see an example in the testsuite patch. ChangeLog 2017-09-10 Tom Tromey * demangle.c (demangle_command): Update. * breakpoint.c (disable_command): Update. (enable_command): Update. (find_location_by_number): Make "number" const. Use get_number_trailer. * cli/cli-utils.c (extract_arg): Return std::string. * probe.c (parse_probe_linespec): Update. Change types. (collect_probes): Take string arguments. (parse_probe_linespec): Likewise. (info_probes_for_ops): Update. (enable_probes_command): Update. (disable_probes_command): Update. * break-catch-sig.c (catch_signal_split_args): Update. * mi/mi-parse.c (mi_parse): Update. testsuite/ChangeLog 2017-09-10 Tom Tromey * gdb.base/ena-dis-br.exp (test_ena_dis_br): Update test. --- gdb/ChangeLog | 17 ++++++++++++ gdb/break-catch-sig.c | 12 ++++----- gdb/breakpoint.c | 39 ++++++++++++++------------- gdb/cli/cli-utils.c | 14 +++++----- gdb/cli/cli-utils.h | 13 ++++----- gdb/demangle.c | 14 +++++----- gdb/mi/mi-parse.c | 12 +++------ gdb/probe.c | 50 +++++++++++++++-------------------- gdb/testsuite/ChangeLog | 4 +++ gdb/testsuite/gdb.base/ena-dis-br.exp | 2 +- 10 files changed, 91 insertions(+), 86 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4c2a924..3201486 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,5 +1,22 @@ 2017-09-10 Tom Tromey + * demangle.c (demangle_command): Update. + * breakpoint.c (disable_command): Update. + (enable_command): Update. + (find_location_by_number): Make "number" const. Use + get_number_trailer. + * cli/cli-utils.c (extract_arg): Return std::string. + * probe.c (parse_probe_linespec): Update. Change types. + (collect_probes): Take string arguments. + (parse_probe_linespec): Likewise. + (info_probes_for_ops): Update. + (enable_probes_command): Update. + (disable_probes_command): Update. + * break-catch-sig.c (catch_signal_split_args): Update. + * mi/mi-parse.c (mi_parse): Update. + +2017-09-10 Tom Tromey + * language.h (language_enum): Make argument const. * language.c (language_enum): Make argument const. diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c index 805084f..237dbaa 100644 --- a/gdb/break-catch-sig.c +++ b/gdb/break-catch-sig.c @@ -343,12 +343,12 @@ catch_signal_split_args (char *arg, bool *catch_all) gdb_signal signal_number; char *endptr; - gdb::unique_xmalloc_ptr one_arg (extract_arg (&arg)); - if (one_arg == NULL) + std::string one_arg = extract_arg (&arg); + if (one_arg.empty ()) break; /* Check for the special flag "all". */ - if (strcmp (one_arg.get (), "all") == 0) + if (strcmp (one_arg.c_str (), "all") == 0) { arg = skip_spaces (arg); if (*arg != '\0' || !first) @@ -361,14 +361,14 @@ catch_signal_split_args (char *arg, bool *catch_all) first = false; /* Check if the user provided a signal name or a number. */ - num = (int) strtol (one_arg.get (), &endptr, 0); + num = (int) strtol (one_arg.c_str (), &endptr, 0); if (*endptr == '\0') signal_number = gdb_signal_from_command (num); else { - signal_number = gdb_signal_from_name (one_arg.get ()); + signal_number = gdb_signal_from_name (one_arg.c_str ()); if (signal_number == GDB_SIGNAL_UNKNOWN) - error (_("Unknown signal name '%s'."), one_arg.get ()); + error (_("Unknown signal name '%s'."), one_arg.c_str ()); } result.push_back (signal_number); diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index b642bc6..79543fe 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -14489,20 +14489,17 @@ map_breakpoint_numbers (const char *args, } static struct bp_location * -find_location_by_number (char *number) +find_location_by_number (const char *number) { - char *dot = strchr (number, '.'); - char *p1; + const char *p1; int bp_num; int loc_num; struct breakpoint *b; struct bp_location *loc; - *dot = '\0'; - p1 = number; - bp_num = get_number (&p1); - if (bp_num == 0) + bp_num = get_number_trailer (&p1, '.'); + if (bp_num == 0 || p1[0] != '.') error (_("Bad breakpoint number '%s'"), number); ALL_BREAKPOINTS (b) @@ -14514,7 +14511,9 @@ find_location_by_number (char *number) if (!b || b->number != bp_num) error (_("Bad breakpoint number '%s'"), number); - p1 = dot+1; + /* Skip the dot. */ + ++p1; + const char *save = p1; loc_num = get_number (&p1); if (loc_num == 0) error (_("Bad breakpoint location number '%s'"), number); @@ -14524,7 +14523,7 @@ find_location_by_number (char *number) for (;loc_num && loc; --loc_num, loc = loc->next) ; if (!loc) - error (_("Bad breakpoint location number '%s'"), dot+1); + error (_("Bad breakpoint location number '%s'"), save); return loc; } @@ -14592,13 +14591,13 @@ disable_command (char *args, int from_tty) } else { - char *num = extract_arg (&args); + std::string num = extract_arg (&args); - while (num) + while (!num.empty ()) { - if (strchr (num, '.')) + if (num.find ('.') != std::string::npos) { - struct bp_location *loc = find_location_by_number (num); + struct bp_location *loc = find_location_by_number (num.c_str ()); if (loc) { @@ -14615,7 +14614,8 @@ disable_command (char *args, int from_tty) update_global_location_list (UGLL_DONT_INSERT); } else - map_breakpoint_numbers (num, do_map_disable_breakpoint, NULL); + map_breakpoint_numbers (num.c_str (), do_map_disable_breakpoint, + NULL); num = extract_arg (&args); } } @@ -14723,13 +14723,13 @@ enable_command (char *args, int from_tty) } else { - char *num = extract_arg (&args); + std::string num = extract_arg (&args); - while (num) + while (!num.empty ()) { - if (strchr (num, '.')) + if (num.find ('.') != std::string::npos) { - struct bp_location *loc = find_location_by_number (num); + struct bp_location *loc = find_location_by_number (num.c_str ()); if (loc) { @@ -14746,7 +14746,8 @@ enable_command (char *args, int from_tty) update_global_location_list (UGLL_MAY_INSERT); } else - map_breakpoint_numbers (num, do_map_enable_breakpoint, NULL); + map_breakpoint_numbers (num.c_str (), do_map_enable_breakpoint, + NULL); num = extract_arg (&args); } } diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c index a00bc52..d5273b5 100644 --- a/gdb/cli/cli-utils.c +++ b/gdb/cli/cli-utils.c @@ -249,36 +249,36 @@ remove_trailing_whitespace (const char *start, const char *s) /* See documentation in cli-utils.h. */ -char * +std::string extract_arg (const char **arg) { const char *result; if (!*arg) - return NULL; + return std::string (); /* Find the start of the argument. */ *arg = skip_spaces (*arg); if (!**arg) - return NULL; + return std::string (); result = *arg; /* Find the end of the argument. */ *arg = skip_to_space (*arg + 1); if (result == *arg) - return NULL; + return std::string (); - return savestring (result, *arg - result); + return std::string (result, *arg - result); } /* See documentation in cli-utils.h. */ -char * +std::string extract_arg (char **arg) { const char *arg_const = *arg; - char *result; + std::string result; result = extract_arg (&arg_const); *arg += arg_const - *arg; diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h index 038ddad..c17b4dd 100644 --- a/gdb/cli/cli-utils.h +++ b/gdb/cli/cli-utils.h @@ -149,17 +149,14 @@ remove_trailing_whitespace (const char *start, char *s) } /* A helper function to extract an argument from *ARG. An argument is - delimited by whitespace. The return value is either NULL if no - argument was found, or an xmalloc'd string. */ + delimited by whitespace. The return value is either empty if no + argument was found. */ -extern char *extract_arg (char **arg); +extern std::string extract_arg (char **arg); -/* A const-correct version of "extract_arg". +/* A const-correct version of "extract_arg". */ - Since the returned value is xmalloc'd, it eventually needs to be - xfree'ed, which prevents us from making it const as well. */ - -extern char *extract_arg (const char **arg); +extern std::string extract_arg (const char **arg); /* A helper function that looks for an argument at the start of a string. The argument must also either be at the end of the string, diff --git a/gdb/demangle.c b/gdb/demangle.c index 892ad18..5430fb6 100644 --- a/gdb/demangle.c +++ b/gdb/demangle.c @@ -170,21 +170,21 @@ demangle_command (char *args, int from_tty) std::string arg_buf = args != NULL ? args : ""; arg_start = arg_buf.c_str (); - gdb::unique_xmalloc_ptr lang_name; + std::string lang_name; while (processing_args && *arg_start == '-') { const char *p = skip_to_space (arg_start); if (strncmp (arg_start, "-l", p - arg_start) == 0) - lang_name.reset (extract_arg (&p)); + lang_name = extract_arg (&p); else if (strncmp (arg_start, "--", p - arg_start) == 0) processing_args = 0; else { - gdb::unique_xmalloc_ptr option (extract_arg (&p)); + std::string option = extract_arg (&p); error (_("Unrecognized option '%s' to demangle command. " - "Try \"help demangle\"."), option.get ()); + "Try \"help demangle\"."), option.c_str ()); } arg_start = skip_spaces (p); @@ -195,13 +195,13 @@ demangle_command (char *args, int from_tty) if (*name == '\0') error (_("Usage: demangle [-l language] [--] name")); - if (lang_name != NULL) + if (!lang_name.empty ()) { enum language lang_enum; - lang_enum = language_enum (lang_name.get ()); + lang_enum = language_enum (lang_name.c_str ()); if (lang_enum == language_unknown) - error (_("Unknown language \"%s\""), lang_name.get ()); + error (_("Unknown language \"%s\""), lang_name.c_str ()); lang = language_def (lang_enum); } else diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c index ad1478e..cf05fa0 100644 --- a/gdb/mi/mi-parse.c +++ b/gdb/mi/mi-parse.c @@ -347,20 +347,14 @@ mi_parse (const char *cmd, char **token) } else if (strncmp (chp, "--language ", ls) == 0) { - char *lang_name; - struct cleanup *old_chain; - option = "--language"; chp += ls; - lang_name = extract_arg (&chp); - old_chain = make_cleanup (xfree, lang_name); + std::string lang_name = extract_arg (&chp); - parse->language = language_enum (lang_name); + parse->language = language_enum (lang_name.c_str ()); if (parse->language == language_unknown || parse->language == language_auto) - error (_("Invalid --language argument: %s"), lang_name); - - do_cleanups (old_chain); + error (_("Invalid --language argument: %s"), lang_name.c_str ()); } else break; diff --git a/gdb/probe.c b/gdb/probe.c index 7b3b888..11fc775 100644 --- a/gdb/probe.c +++ b/gdb/probe.c @@ -275,8 +275,8 @@ find_probe_by_pc (CORE_ADDR pc) Each argument is a regexp, or NULL, which matches anything. */ static VEC (bound_probe_s) * -collect_probes (char *objname, char *provider, char *probe_name, - const struct probe_ops *pops) +collect_probes (const std::string &objname, const std::string &provider, + const std::string &probe_name, const struct probe_ops *pops) { struct objfile *objfile; VEC (bound_probe_s) *result = NULL; @@ -285,12 +285,15 @@ collect_probes (char *objname, char *provider, char *probe_name, cleanup = make_cleanup (VEC_cleanup (bound_probe_s), &result); - if (provider != NULL) - prov_pat.emplace (provider, REG_NOSUB, _("Invalid provider regexp")); - if (probe_name != NULL) - probe_pat.emplace (probe_name, REG_NOSUB, _("Invalid probe regexp")); - if (objname != NULL) - obj_pat.emplace (objname, REG_NOSUB, _("Invalid object file regexp")); + if (!provider.empty ()) + prov_pat.emplace (provider.c_str (), REG_NOSUB, + _("Invalid provider regexp")); + if (!probe_name.empty ()) + probe_pat.emplace (probe_name.c_str (), REG_NOSUB, + _("Invalid probe regexp")); + if (!objname.empty ()) + obj_pat.emplace (objname.c_str (), REG_NOSUB, + _("Invalid object file regexp")); ALL_OBJFILES (objfile) { @@ -301,7 +304,7 @@ collect_probes (char *objname, char *provider, char *probe_name, if (! objfile->sf || ! objfile->sf->sym_probe_fns) continue; - if (objname) + if (obj_pat) { if (obj_pat->exec (objfile_name (objfile), 0, NULL, 0) != 0) continue; @@ -316,11 +319,11 @@ collect_probes (char *objname, char *provider, char *probe_name, if (pops != NULL && probe->pops != pops) continue; - if (provider + if (prov_pat && prov_pat->exec (probe->provider, 0, NULL, 0) != 0) continue; - if (probe_name + if (probe_pat && probe_pat->exec (probe->name, 0, NULL, 0) != 0) continue; @@ -553,16 +556,14 @@ exists_probe_with_pops (VEC (bound_probe_s) *probes, [PROBE [OBJNAME]]] from the provided string STR. */ static void -parse_probe_linespec (const char *str, char **provider, - char **probe_name, char **objname) +parse_probe_linespec (const char *str, std::string *provider, + std::string *probe_name, std::string *objname) { - *probe_name = *objname = NULL; - *provider = extract_arg (&str); - if (*provider != NULL) + if (!provider->empty ()) { *probe_name = extract_arg (&str); - if (*probe_name != NULL) + if (!probe_name->empty ()) *objname = extract_arg (&str); } } @@ -573,7 +574,7 @@ void info_probes_for_ops (const char *arg, int from_tty, const struct probe_ops *pops) { - char *provider, *probe_name = NULL, *objname = NULL; + std::string provider, probe_name, objname; struct cleanup *cleanup = make_cleanup (null_cleanup, NULL); VEC (bound_probe_s) *probes; int i, any_found; @@ -587,9 +588,6 @@ info_probes_for_ops (const char *arg, int from_tty, struct gdbarch *gdbarch = get_current_arch (); parse_probe_linespec (arg, &provider, &probe_name, &objname); - make_cleanup (xfree, provider); - make_cleanup (xfree, probe_name); - make_cleanup (xfree, objname); probes = collect_probes (objname, provider, probe_name, pops); make_cleanup (VEC_cleanup (probe_p), &probes); @@ -721,16 +719,13 @@ info_probes_command (char *arg, int from_tty) static void enable_probes_command (char *arg, int from_tty) { - char *provider, *probe_name = NULL, *objname = NULL; + std::string provider, probe_name, objname; struct cleanup *cleanup = make_cleanup (null_cleanup, NULL); VEC (bound_probe_s) *probes; struct bound_probe *probe; int i; parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname); - make_cleanup (xfree, provider); - make_cleanup (xfree, probe_name); - make_cleanup (xfree, objname); probes = collect_probes (objname, provider, probe_name, NULL); if (VEC_empty (bound_probe_s, probes)) @@ -765,16 +760,13 @@ enable_probes_command (char *arg, int from_tty) static void disable_probes_command (char *arg, int from_tty) { - char *provider, *probe_name = NULL, *objname = NULL; + std::string provider, probe_name, objname; struct cleanup *cleanup = make_cleanup (null_cleanup, NULL); VEC (bound_probe_s) *probes; struct bound_probe *probe; int i; parse_probe_linespec ((const char *) arg, &provider, &probe_name, &objname); - make_cleanup (xfree, provider); - make_cleanup (xfree, probe_name); - make_cleanup (xfree, objname); probes = collect_probes (objname, provider, probe_name, NULL /* pops */); if (VEC_empty (bound_probe_s, probes)) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 8d8dc3c..d730fda 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2017-09-10 Tom Tromey + + * gdb.base/ena-dis-br.exp (test_ena_dis_br): Update test. + 2017-09-08 Christoph Weinmann * gdb.fortran/printing-types.exp: New file. diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp index 8abca35..d407408 100644 --- a/gdb/testsuite/gdb.base/ena-dis-br.exp +++ b/gdb/testsuite/gdb.base/ena-dis-br.exp @@ -369,7 +369,7 @@ proc test_ena_dis_br { what } { # Now enable(disable) $b1 fooo.1, it should give error on fooo. gdb_test "$what $b1 fooo.1" \ - "Bad breakpoint number 'fooo'" \ + "Bad breakpoint number 'fooo\\.1'" \ "$what \$b1 fooo.1" # $b1 should be enabled(disabled).