From 8e481c3ba86e512b39b16b41de24e87a17f7d968 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 22 Nov 2017 20:17:28 -0700 Subject: [PATCH] C++-ify parse_format_string 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-12-08 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-12-08 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/ChangeLog b/gdb/ChangeLog index 56acf30..21ddce0 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,19 @@ +2017-12-08 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. + 2017-12-08 Yao Qi PR breakpionts/22567 diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c index 5027f6a..5a2a0a0 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)); @@ -2681,12 +2680,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 @@ -2705,9 +2700,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; @@ -2723,15 +2716,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; @@ -2742,14 +2734,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 8b5ab46..834ddff 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 59a4dad..b4353d2 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -2226,12 +2226,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) @@ -2248,9 +2244,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; @@ -2268,17 +2262,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; @@ -2292,7 +2283,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) { @@ -2302,8 +2293,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 8cb1551..95cb805 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 f3a94b8..dd083f9 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/ChangeLog b/gdb/gdbserver/ChangeLog index c84dcac..b5667bf 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,7 @@ +2017-12-08 Tom Tromey + + * ax.c (ax_printf): Update. + 2017-12-07 Yao Qi * linux-aarch64-ipa.c (initialize_low_tracepoint): Call diff --git a/gdb/gdbserver/ax.c b/gdb/gdbserver/ax.c index 35ed2c6..7e5a409 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 2e596d1..7ca8623 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. */ -- 2.7.4