From patchwork Thu Nov 23 16:46:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 24478 Received: (qmail 119283 invoked by alias); 23 Nov 2017 16:46:38 -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 119271 invoked by uid 89); 23 Nov 2017 16:46:38 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KB_WAM_FROM_NAME_SINGLEWORD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=channel, 0s X-HELO: gateway24.websitewelcome.com Received: from gateway24.websitewelcome.com (HELO gateway24.websitewelcome.com) (192.185.51.122) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Nov 2017 16:46:35 +0000 Received: from cm13.websitewelcome.com (cm13.websitewelcome.com [100.42.49.6]) by gateway24.websitewelcome.com (Postfix) with ESMTP id 27278B8F15 for ; Thu, 23 Nov 2017 10:46:34 -0600 (CST) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id Hue2eakixrWstHue2eGU7r; Thu, 23 Nov 2017 10:46:34 -0600 Received: from 71-218-90-63.hlrn.qwest.net ([71.218.90.63]:40796 helo=bapiya.Home) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.89) (envelope-from ) id 1eHue1-002fxc-Sv; Thu, 23 Nov 2017 10:46:34 -0600 From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [RFA] C++-ify parse_format_string Date: Thu, 23 Nov 2017 09:46:31 -0700 Message-Id: <20171123164631.11055-1-tom@tromey.com> X-BWhitelist: no X-Source-L: No X-Exim-ID: 1eHue1-002fxc-Sv X-Source-Sender: 71-218-90-63.hlrn.qwest.net (bapiya.Home) [71.218.90.63]:40796 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes This replaces parse_format_string with a class, removing some constructors along the way. While doing this, I found that one argument to gen_printf is unused, so I removed it. Also, I am not completely sure, but the use of `release' in maint_agent_printf_command and parse_cmd_to_aexpr seems like it may leak expressions. Regression tested by the buildbot. ChangeLog 2017-11-23 Tom Tromey * printcmd.c (ui_printf): Update. Use std::vector. * common/format.h (struct format_piece): Add constructor. : Now const. (class format_pieces): New class. (parse_format_string, free_format_pieces) (free_format_pieces_cleanup): Remove. * common/format.c (format_pieces::format_pieces): Rename from parse_format_string. Update. (free_format_pieces, free_format_pieces_cleanup): Remove. * breakpoint.c (parse_cmd_to_aexpr): Update. Use std::vector. * ax-gdb.h (gen_printf): Remove argument. * ax-gdb.c (gen_printf): Remove "frags" argument. (maint_agent_printf_command): Update. Use std::vector. gdbserver/ChangeLog 2017-11-23 Tom Tromey * ax.c (ax_printf): Update. --- gdb/ChangeLog | 16 +++++++++++++ gdb/ax-gdb.c | 17 ++++---------- gdb/ax-gdb.h | 2 -- gdb/breakpoint.c | 19 ++++----------- gdb/common/format.c | 62 ++++--------------------------------------------- gdb/common/format.h | 42 ++++++++++++++++++++++++++------- gdb/gdbserver/ChangeLog | 4 ++++ gdb/gdbserver/ax.c | 22 ++++++++---------- gdb/printcmd.c | 41 +++++++++++--------------------- 9 files changed, 89 insertions(+), 136 deletions(-) diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c index 52ca081a82..e22e0e6582 100644 --- a/gdb/ax-gdb.c +++ b/gdb/ax-gdb.c @@ -2541,7 +2541,6 @@ agent_expr_up gen_printf (CORE_ADDR scope, struct gdbarch *gdbarch, CORE_ADDR function, LONGEST channel, const char *format, int fmtlen, - struct format_piece *frags, int nargs, struct expression **exprs) { agent_expr_up ax (new agent_expr (gdbarch, scope)); @@ -2680,12 +2679,8 @@ agent_eval_command (const char *exp, int from_tty) static void maint_agent_printf_command (const char *cmdrest, int from_tty) { - struct cleanup *old_chain = 0; - struct expression *argvec[100]; struct frame_info *fi = get_current_frame (); /* need current scope */ const char *format_start, *format_end; - struct format_piece *fpieces; - int nargs; /* We don't deal with overlay debugging at the moment. We need to think more carefully about this. If you copy this code into @@ -2704,9 +2699,7 @@ maint_agent_printf_command (const char *cmdrest, int from_tty) format_start = cmdrest; - fpieces = parse_format_string (&cmdrest); - - old_chain = make_cleanup (free_format_pieces_cleanup, &fpieces); + format_pieces fpieces (&cmdrest); format_end = cmdrest; @@ -2722,15 +2715,14 @@ maint_agent_printf_command (const char *cmdrest, int from_tty) cmdrest++; cmdrest = skip_spaces (cmdrest); - nargs = 0; + std::vector argvec; while (*cmdrest != '\0') { const char *cmd1; cmd1 = cmdrest; expression_up expr = parse_exp_1 (&cmd1, 0, (struct block *) 0, 1); - argvec[nargs] = expr.release (); - ++nargs; + argvec.push_back (expr.release ()); cmdrest = cmd1; if (*cmdrest == ',') ++cmdrest; @@ -2741,14 +2733,13 @@ maint_agent_printf_command (const char *cmdrest, int from_tty) agent_expr_up agent = gen_printf (get_frame_pc (fi), get_current_arch (), 0, 0, format_start, format_end - format_start, - fpieces, nargs, argvec); + argvec.size (), argvec.data ()); ax_reqs (agent.get ()); ax_print (gdb_stdout, agent.get ()); /* It would be nice to call ax_reqs here to gather some general info about the expression, and then print out the result. */ - do_cleanups (old_chain); dont_repeat (); } diff --git a/gdb/ax-gdb.h b/gdb/ax-gdb.h index 8b5ab46c66..834ddffe05 100644 --- a/gdb/ax-gdb.h +++ b/gdb/ax-gdb.h @@ -120,10 +120,8 @@ extern void gen_expr (struct expression *exp, union exp_element **pc, extern void require_rvalue (struct agent_expr *ax, struct axs_value *value); -struct format_piece; extern agent_expr_up gen_printf (CORE_ADDR, struct gdbarch *, CORE_ADDR, LONGEST, const char *, int, - struct format_piece *, int, struct expression **); #endif /* AX_GDB_H */ diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index b48c405b08..41c542e08b 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2230,12 +2230,8 @@ build_target_condition_list (struct bp_location *bl) static agent_expr_up parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) { - struct cleanup *old_cleanups = 0; - struct expression **argvec; const char *cmdrest; const char *format_start, *format_end; - struct format_piece *fpieces; - int nargs; struct gdbarch *gdbarch = get_current_arch (); if (cmd == NULL) @@ -2252,9 +2248,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) format_start = cmdrest; - fpieces = parse_format_string (&cmdrest); - - old_cleanups = make_cleanup (free_format_pieces_cleanup, &fpieces); + format_pieces fpieces (&cmdrest); format_end = cmdrest; @@ -2272,17 +2266,14 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) /* For each argument, make an expression. */ - argvec = (struct expression **) alloca (strlen (cmd) - * sizeof (struct expression *)); - - nargs = 0; + std::vector argvec; while (*cmdrest != '\0') { const char *cmd1; cmd1 = cmdrest; expression_up expr = parse_exp_1 (&cmd1, scope, block_for_pc (scope), 1); - argvec[nargs++] = expr.release (); + argvec.push_back (expr.release ()); cmdrest = cmd1; if (*cmdrest == ',') ++cmdrest; @@ -2296,7 +2287,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) { aexpr = gen_printf (scope, gdbarch, 0, 0, format_start, format_end - format_start, - fpieces, nargs, argvec); + argvec.size (), argvec.data ()); } CATCH (ex, RETURN_MASK_ERROR) { @@ -2306,8 +2297,6 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd) } END_CATCH - do_cleanups (old_cleanups); - /* We have a valid agent expression, return it. */ return aexpr; } diff --git a/gdb/common/format.c b/gdb/common/format.c index 8cb15511fa..95cb8053c4 100644 --- a/gdb/common/format.c +++ b/gdb/common/format.c @@ -20,17 +20,13 @@ #include "common-defs.h" #include "format.h" -struct format_piece * -parse_format_string (const char **arg) +format_pieces::format_pieces (const char **arg) { const char *s; char *f, *string; const char *prev_start; const char *percent_loc; char *sub_start, *current_substring; - struct format_piece *pieces; - int next_frag; - int max_pieces; enum argclass this_argclass; s = *arg; @@ -100,12 +96,7 @@ parse_format_string (const char **arg) /* Need extra space for the '\0's. Doubling the size is sufficient. */ current_substring = (char *) xmalloc (strlen (string) * 2 + 1000); - - max_pieces = strlen (string) + 2; - - pieces = XNEWVEC (struct format_piece, max_pieces); - - next_frag = 0; + m_storage.reset (current_substring); /* Now scan the string for %-specs and see what kinds of args they want. argclass classifies the %-specs so we can give printf-type functions @@ -135,9 +126,7 @@ parse_format_string (const char **arg) current_substring += f - 1 - prev_start; *current_substring++ = '\0'; - pieces[next_frag].string = sub_start; - pieces[next_frag].argclass = literal_piece; - next_frag++; + m_pieces.emplace_back (sub_start, literal_piece); percent_loc = f - 1; @@ -343,9 +332,7 @@ parse_format_string (const char **arg) prev_start = f; - pieces[next_frag].string = sub_start; - pieces[next_frag].argclass = this_argclass; - next_frag++; + m_pieces.emplace_back (sub_start, this_argclass); } /* Record the remainder of the string. */ @@ -356,44 +343,5 @@ parse_format_string (const char **arg) current_substring += f - prev_start; *current_substring++ = '\0'; - pieces[next_frag].string = sub_start; - pieces[next_frag].argclass = literal_piece; - next_frag++; - - /* Record an end-of-array marker. */ - - pieces[next_frag].string = NULL; - pieces[next_frag].argclass = literal_piece; - - return pieces; + m_pieces.emplace_back (sub_start, literal_piece); } - -void -free_format_pieces (struct format_piece *pieces) -{ - if (!pieces) - return; - - /* We happen to know that all the string pieces are in the block - pointed to by the first string piece. */ - if (pieces[0].string) - xfree (pieces[0].string); - - xfree (pieces); -} - -void -free_format_pieces_cleanup (void *ptr) -{ - struct format_piece **location = (struct format_piece **) ptr; - - if (location == NULL) - return; - - if (*location != NULL) - { - free_format_pieces (*location); - *location = NULL; - } -} - diff --git a/gdb/common/format.h b/gdb/common/format.h index f3a94b8bbb..dd083f9ac1 100644 --- a/gdb/common/format.h +++ b/gdb/common/format.h @@ -48,22 +48,46 @@ enum argclass struct format_piece { - char *string; + format_piece (const char *str, enum argclass argc) + : string (str), + argclass (argc) + { + } + + const char *string; enum argclass argclass; }; -/* Return an array of printf fragments found at the given string, and - rewrite ARG with a pointer to the end of the format string. */ +class format_pieces +{ +public: + + format_pieces (const char **arg); + ~format_pieces () = default; + + DISABLE_COPY_AND_ASSIGN (format_pieces); -extern struct format_piece *parse_format_string (const char **arg); + format_piece &operator[] (size_t index) + { + return m_pieces[index]; + } -/* Given a pointer to an array of format pieces, free any memory that - would have been allocated by parse_format_string. */ + typedef std::vector::iterator iterator; -extern void free_format_pieces (struct format_piece *frags); + iterator begin () + { + return m_pieces.begin (); + } -/* Freeing, cast as a cleanup. */ + iterator end () + { + return m_pieces.end (); + } -extern void free_format_pieces_cleanup (void *); +private: + + std::vector m_pieces; + gdb::unique_xmalloc_ptr m_storage; +}; #endif /* COMMON_FORMAT_H */ diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c index 35ed2c69ad..7e5a409cfd 100644 --- a/gdb/gdbserver/ax.c +++ b/gdb/gdbserver/ax.c @@ -816,30 +816,29 @@ ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format, int nargs, ULONGEST *args) { const char *f = format; - struct format_piece *fpieces; - int i, fp; - char *current_substring; + int i; + const char *current_substring; int nargs_wanted; ax_debug ("Printf of \"%s\" with %d args", format, nargs); - fpieces = parse_format_string (&f); + format_pieces fpieces (&f); nargs_wanted = 0; - for (fp = 0; fpieces[fp].string != NULL; fp++) - if (fpieces[fp].argclass != literal_piece) + for (auto &&piece : fpieces) + if (piece.argclass != literal_piece) ++nargs_wanted; if (nargs != nargs_wanted) error (_("Wrong number of arguments for specified format-string")); i = 0; - for (fp = 0; fpieces[fp].string != NULL; fp++) + for (auto &&piece : fpieces) { - current_substring = fpieces[fp].string; + current_substring = piece.string; ax_debug ("current substring is '%s', class is %d", - current_substring, fpieces[fp].argclass); - switch (fpieces[fp].argclass) + current_substring, piece.argclass); + switch (piece.argclass) { case string_arg: { @@ -914,11 +913,10 @@ ax_printf (CORE_ADDR fn, CORE_ADDR chan, const char *format, } /* Maybe advance to the next argument. */ - if (fpieces[fp].argclass != literal_piece) + if (piece.argclass != literal_piece) ++i; } - free_format_pieces (fpieces); fflush (stdout); } diff --git a/gdb/printcmd.c b/gdb/printcmd.c index 2e596d1f09..7ca86232a1 100644 --- a/gdb/printcmd.c +++ b/gdb/printcmd.c @@ -2427,14 +2427,8 @@ printf_pointer (struct ui_file *stream, const char *format, static void ui_printf (const char *arg, struct ui_file *stream) { - struct format_piece *fpieces; const char *s = arg; - struct value **val_args; - int allocated_args = 20; - struct cleanup *old_cleanups; - - val_args = XNEWVEC (struct value *, allocated_args); - old_cleanups = make_cleanup (free_current_contents, &val_args); + std::vector val_args; if (s == 0) error_no_arg (_("format-control string and values to print")); @@ -2445,9 +2439,7 @@ ui_printf (const char *arg, struct ui_file *stream) if (*s++ != '"') error (_("Bad format string, missing '\"'.")); - fpieces = parse_format_string (&s); - - make_cleanup (free_format_pieces_cleanup, &fpieces); + format_pieces fpieces (&s); if (*s++ != '"') error (_("Bad format string, non-terminated '\"'.")); @@ -2462,14 +2454,13 @@ ui_printf (const char *arg, struct ui_file *stream) s = skip_spaces (s); { - int nargs = 0; int nargs_wanted; - int i, fr; - char *current_substring; + int i; + const char *current_substring; nargs_wanted = 0; - for (fr = 0; fpieces[fr].string != NULL; fr++) - if (fpieces[fr].argclass != literal_piece) + for (auto &&piece : fpieces) + if (piece.argclass != literal_piece) ++nargs_wanted; /* Now, parse all arguments and evaluate them. @@ -2479,28 +2470,23 @@ ui_printf (const char *arg, struct ui_file *stream) { const char *s1; - if (nargs == allocated_args) - val_args = (struct value **) xrealloc ((char *) val_args, - (allocated_args *= 2) - * sizeof (struct value *)); s1 = s; - val_args[nargs] = parse_to_comma_and_eval (&s1); + val_args.push_back (parse_to_comma_and_eval (&s1)); - nargs++; s = s1; if (*s == ',') s++; } - if (nargs != nargs_wanted) + if (val_args.size () != nargs_wanted) error (_("Wrong number of arguments for specified format-string")); /* Now actually print them. */ i = 0; - for (fr = 0; fpieces[fr].string != NULL; fr++) + for (auto &&piece : fpieces) { - current_substring = fpieces[fr].string; - switch (fpieces[fr].argclass) + current_substring = piece.string; + switch (piece.argclass) { case string_arg: printf_c_string (stream, current_substring, val_args[i]); @@ -2569,7 +2555,7 @@ ui_printf (const char *arg, struct ui_file *stream) case dec64float_arg: case dec128float_arg: printf_floating (stream, current_substring, val_args[i], - fpieces[fr].argclass); + piece.argclass); break; case ptr_arg: printf_pointer (stream, current_substring, val_args[i]); @@ -2590,11 +2576,10 @@ ui_printf (const char *arg, struct ui_file *stream) _("failed internal consistency check")); } /* Maybe advance to the next argument. */ - if (fpieces[fr].argclass != literal_piece) + if (piece.argclass != literal_piece) ++i; } } - do_cleanups (old_cleanups); } /* Implement the "printf" command. */