[RFA,v2,10/17] C++ify mi_parse
Commit Message
This changes mi_parse to return a unique_ptr, and to use "new"; then
fixes up the users. This allows removing one cleanup.
gdb/ChangeLog
2017-04-11 Tom Tromey <tom@tromey.com>
* mi/mi-parse.h (struct mi_parse): Add constructor, destructor.
(mi_parse): Update return type..
(mi_parse_free): Remove.
* mi/mi-parse.c (mi_parse::~mi_parse): Rename from mi_parse_free.
(mi_parse_cleanup): Remove.
(mi_parse): Return a unique_ptr. Use new.
* mi/mi-main.c (mi_execute_command): Update.
---
gdb/ChangeLog | 10 ++++++++++
gdb/mi/mi-main.c | 8 +++-----
gdb/mi/mi-parse.c | 40 +++++++++-------------------------------
gdb/mi/mi-parse.h | 14 +++++++++-----
4 files changed, 31 insertions(+), 41 deletions(-)
Comments
On 04/11/2017 04:01 PM, Tom Tromey wrote:
> - memset (parse, 0, sizeof (*parse));
> + std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
> + memset (parse.get (), 0, sizeof (struct mi_parse));
> parse->all = 0;
The patch made mi_parse be a non-POD (non-trivial dtor), so I
think we should get rid of that memset at the same time.
Thanks,
Pedro Alves
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> The patch made mi_parse be a non-POD (non-trivial dtor), so I
Pedro> think we should get rid of that memset at the same time.
This change was a little bigger than the others. I've added this
constructor:
mi_parse::mi_parse ()
: op (MI_COMMAND),
command (NULL),
token (NULL),
cmd (NULL),
cmd_start (NULL),
args (NULL),
argv (NULL),
argc (0),
all (0),
thread_group (-1),
thread (-1),
frame (-1),
language (language_unknown)
{
}
... and changed the construction of the object like:
@@ -241,12 +241,5 @@
- memset (parse, 0, sizeof (*parse));
- parse->all = 0;
- parse->thread_group = -1;
- parse->thread = -1;
- parse->frame = -1;
- parse->language = language_unknown;
-
- cleanup = make_cleanup (mi_parse_cleanup, parse);
+ std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
/* Before starting, skip leading white space. */
cmd = skip_spaces_const (cmd);
Tom
On 04/12/2017 05:15 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> The patch made mi_parse be a non-POD (non-trivial dtor), so I
> Pedro> think we should get rid of that memset at the same time.
>
> This change was a little bigger than the others. I've added this
> constructor:
>
> mi_parse::mi_parse ()
> : op (MI_COMMAND),
> command (NULL),
> token (NULL),
> cmd (NULL),
> cmd_start (NULL),
> args (NULL),
> argv (NULL),
> argc (0),
> all (0),
> thread_group (-1),
> thread (-1),
> frame (-1),
> language (language_unknown)
> {
> }
>
Thanks. Totally fine with me. If you prefer, using in-class
initialization is also fine. (We've been using it more in recent patches.)
> ... and changed the construction of the object like:
>
> @@ -241,12 +241,5 @@
>
> - memset (parse, 0, sizeof (*parse));
> - parse->all = 0;
> - parse->thread_group = -1;
> - parse->thread = -1;
> - parse->frame = -1;
> - parse->language = language_unknown;
> -
> - cleanup = make_cleanup (mi_parse_cleanup, parse);
> + std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
>
Perfect.
Thanks,
Pedro Alves
Pedro> Thanks. Totally fine with me. If you prefer, using in-class
Pedro> initialization is also fine. (We've been using it more in recent
Pedro> patches.)
I just left it as-is, but not for any good reason.
Tom
@@ -1,5 +1,15 @@
2017-04-11 Tom Tromey <tom@tromey.com>
+ * mi/mi-parse.h (struct mi_parse): Add constructor, destructor.
+ (mi_parse): Update return type..
+ (mi_parse_free): Remove.
+ * mi/mi-parse.c (mi_parse::~mi_parse): Rename from mi_parse_free.
+ (mi_parse_cleanup): Remove.
+ (mi_parse): Return a unique_ptr. Use new.
+ * mi/mi-main.c (mi_execute_command): Update.
+
+2017-04-11 Tom Tromey <tom@tromey.com>
+
* location.c (explicit_location_lex_one): Return a
unique_xmalloc_ptr.
(string_to_explicit_location): Update. Remove cleanups.
@@ -2117,7 +2117,7 @@ void
mi_execute_command (const char *cmd, int from_tty)
{
char *token;
- struct mi_parse *command = NULL;
+ std::unique_ptr<struct mi_parse> command;
/* This is to handle EOF (^D). We just quit gdb. */
/* FIXME: we should call some API function here. */
@@ -2158,7 +2158,7 @@ mi_execute_command (const char *cmd, int from_tty)
TRY
{
- captured_mi_execute_command (current_uiout, command);
+ captured_mi_execute_command (current_uiout, command.get ());
}
CATCH (result, RETURN_MASK_ALL)
{
@@ -2186,7 +2186,7 @@ mi_execute_command (const char *cmd, int from_tty)
&& thread_count () != 0
/* If the command already reports the thread change, no need to do it
again. */
- && !command_notifies_uscc_observer (command))
+ && !command_notifies_uscc_observer (command.get ()))
{
struct mi_interp *mi = (struct mi_interp *) top_level_interpreter ();
int report_change = 0;
@@ -2211,8 +2211,6 @@ mi_execute_command (const char *cmd, int from_tty)
}
}
- mi_parse_free (command);
-
do_cleanups (cleanup);
}
}
@@ -208,46 +208,28 @@ mi_parse_argv (const char *args, struct mi_parse *parse)
}
}
-void
-mi_parse_free (struct mi_parse *parse)
+mi_parse::~mi_parse ()
{
- if (parse == NULL)
- return;
- if (parse->command != NULL)
- xfree (parse->command);
- if (parse->token != NULL)
- xfree (parse->token);
- if (parse->args != NULL)
- xfree (parse->args);
- if (parse->argv != NULL)
- freeargv (parse->argv);
- xfree (parse);
+ xfree (command);
+ xfree (token);
+ xfree (args);
+ freeargv (argv);
}
-/* A cleanup that calls mi_parse_free. */
-
-static void
-mi_parse_cleanup (void *arg)
-{
- mi_parse_free ((struct mi_parse *) arg);
-}
-
-struct mi_parse *
+std::unique_ptr<struct mi_parse>
mi_parse (const char *cmd, char **token)
{
const char *chp;
- struct mi_parse *parse = XNEW (struct mi_parse);
struct cleanup *cleanup;
- memset (parse, 0, sizeof (*parse));
+ std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
+ memset (parse.get (), 0, sizeof (struct mi_parse));
parse->all = 0;
parse->thread_group = -1;
parse->thread = -1;
parse->frame = -1;
parse->language = language_unknown;
- cleanup = make_cleanup (mi_parse_cleanup, parse);
-
/* Before starting, skip leading white space. */
cmd = skip_spaces_const (cmd);
@@ -265,8 +247,6 @@ mi_parse (const char *cmd, char **token)
parse->command = xstrdup (chp);
parse->op = CLI_COMMAND;
- discard_cleanups (cleanup);
-
return parse;
}
@@ -383,7 +363,7 @@ mi_parse (const char *cmd, char **token)
list. */
if (parse->cmd->argv_func != NULL)
{
- mi_parse_argv (chp, parse);
+ mi_parse_argv (chp, parse.get ());
if (parse->argv == NULL)
error (_("Problem parsing arguments: %s %s"), parse->command, chp);
}
@@ -394,8 +374,6 @@ mi_parse (const char *cmd, char **token)
if (parse->cmd->cli.cmd != NULL)
parse->args = xstrdup (chp);
- discard_cleanups (cleanup);
-
/* Fully parsed, flag as an MI command. */
parse->op = MI_COMMAND;
return parse;
@@ -41,6 +41,13 @@ enum mi_command_type
struct mi_parse
{
+ mi_parse () = default;
+
+ ~mi_parse ();
+
+ mi_parse (const mi_parse &) = delete;
+ mi_parse &operator= (const mi_parse &) = delete;
+
enum mi_command_type op;
char *command;
char *token;
@@ -67,11 +74,8 @@ struct mi_parse
the TOKEN field of the resultant mi_parse object, to be freed by
mi_parse_free. */
-extern struct mi_parse *mi_parse (const char *cmd, char **token);
-
-/* Free a command returned by mi_parse_command. */
-
-extern void mi_parse_free (struct mi_parse *cmd);
+extern std::unique_ptr<struct mi_parse> mi_parse (const char *cmd,
+ char **token);
/* Parse a string argument into a print_values value. */