Simplify macro_define_command
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
This simplifies macro_define_command, using std::vector to remove some
manual memory management. temporary_macro_definition is also
simplified -- much of the code there was simply unnecessary.
Regression tested on x86-64 Fedora 38.
---
gdb/macrocmd.c | 55 +++++++++++++++-----------------------------------
1 file changed, 16 insertions(+), 39 deletions(-)
Comments
On 4/22/24 11:49 AM, Tom Tromey wrote:
> This simplifies macro_define_command, using std::vector to remove some
> manual memory management. temporary_macro_definition is also
> simplified -- much of the code there was simply unnecessary.
>
> Regression tested on x86-64 Fedora 38.
> ---
> gdb/macrocmd.c | 55 +++++++++++++++-----------------------------------
> 1 file changed, 16 insertions(+), 39 deletions(-)
>
> diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
> index ddabfede0f5..0d0db5d1b90 100644
> --- a/gdb/macrocmd.c
> +++ b/gdb/macrocmd.c
> @@ -308,33 +308,24 @@ extract_identifier (const char **expp, int is_parameter)
> return gdb::unique_xmalloc_ptr<char> (result);
> }
>
> -struct temporary_macro_definition : public macro_definition
> +struct temporary_macro_definition
> {
> - temporary_macro_definition ()
> + ~temporary_macro_definition ()
> {
> - table = nullptr;
> - kind = macro_object_like;
> - argc = 0;
> - argv = nullptr;
> - replacement = nullptr;
> + free_vector_argv (argv);
> }
Since this structure has a destructor, perhaps make it
non-copyable/assignable/etc (rule of 3/5)?
It could be a subsequent cleanup, but that class could have a more
general name and live in gdbsupport/common-utils.h. It could then be
used by other current users of free_vector_argv.
> @@ -358,26 +343,18 @@ macro_define_command (const char *exp, int from_tty)
>
> while (*exp != ')')
> {
> - int i;
> -
> - if (new_macro.argc == alloced)
> - {
> - alloced *= 2;
> - argv = (char **) xrealloc (argv, alloced * sizeof (char *));
> - /* Must update new_macro as well... */
> - new_macro.argv = (const char * const *) argv;
> - }
> - argv[new_macro.argc] = extract_identifier (&exp, 1).release ();
> - if (! argv[new_macro.argc])
> + auto arg = extract_identifier (&exp, 1);
> + if (! arg)
arg == nullptr
> error (_("Macro is missing an argument."));
> - ++new_macro.argc;
>
> - for (i = new_macro.argc - 2; i >= 0; --i)
> + for (const char *existing : new_macro.argv)
> {
> - if (! strcmp (argv[i], argv[new_macro.argc - 1]))
> + if (! strcmp (existing, arg.get ()))
> error (_("Two macro arguments with identical names."));
> }
You could remove the braces while at it.
Simon
@@ -308,33 +308,24 @@ extract_identifier (const char **expp, int is_parameter)
return gdb::unique_xmalloc_ptr<char> (result);
}
-struct temporary_macro_definition : public macro_definition
+struct temporary_macro_definition
{
- temporary_macro_definition ()
+ ~temporary_macro_definition ()
{
- table = nullptr;
- kind = macro_object_like;
- argc = 0;
- argv = nullptr;
- replacement = nullptr;
+ free_vector_argv (argv);
}
- ~temporary_macro_definition ()
+ void add_argument (gdb::unique_xmalloc_ptr<char> &&value)
{
- int i;
-
- for (i = 0; i < argc; ++i)
- xfree ((char *) argv[i]);
- xfree ((char *) argv);
- /* Note that the 'replacement' field is not allocated. */
+ argv.push_back (value.release ());
}
+
+ std::vector<char *> argv;
};
static void
macro_define_command (const char *exp, int from_tty)
{
- temporary_macro_definition new_macro;
-
if (!exp)
error (_("usage: macro define NAME[(ARGUMENT-LIST)] [REPLACEMENT-LIST]"));
@@ -344,13 +335,7 @@ macro_define_command (const char *exp, int from_tty)
error (_("Invalid macro name."));
if (*exp == '(')
{
- /* Function-like macro. */
- int alloced = 5;
- char **argv = XNEWVEC (char *, alloced);
-
- new_macro.kind = macro_function_like;
- new_macro.argc = 0;
- new_macro.argv = (const char * const *) argv;
+ temporary_macro_definition new_macro;
/* Skip the '(' and whitespace. */
++exp;
@@ -358,26 +343,18 @@ macro_define_command (const char *exp, int from_tty)
while (*exp != ')')
{
- int i;
-
- if (new_macro.argc == alloced)
- {
- alloced *= 2;
- argv = (char **) xrealloc (argv, alloced * sizeof (char *));
- /* Must update new_macro as well... */
- new_macro.argv = (const char * const *) argv;
- }
- argv[new_macro.argc] = extract_identifier (&exp, 1).release ();
- if (! argv[new_macro.argc])
+ auto arg = extract_identifier (&exp, 1);
+ if (! arg)
error (_("Macro is missing an argument."));
- ++new_macro.argc;
- for (i = new_macro.argc - 2; i >= 0; --i)
+ for (const char *existing : new_macro.argv)
{
- if (! strcmp (argv[i], argv[new_macro.argc - 1]))
+ if (! strcmp (existing, arg.get ()))
error (_("Two macro arguments with identical names."));
}
+ new_macro.add_argument (std::move (arg));
+
skip_ws (&exp);
if (*exp == ',')
{
@@ -392,8 +369,8 @@ macro_define_command (const char *exp, int from_tty)
skip_ws (&exp);
macro_define_function (macro_main (macro_user_macros), -1, name.get (),
- new_macro.argc, (const char **) new_macro.argv,
- exp);
+ new_macro.argv.size (),
+ (const char **) new_macro.argv.data (), exp);
}
else
{