GDB 7.99.91 available for testing
Commit Message
On 17-05-16 09:51:42, Simon Marchi wrote:
>
> So indeed, the problem is due to the ordering of the _initialize functions
> that changed, _initialize_printcmd used to be called before
> _initialize_infcmd, now it's the reverse.
>
> When it works, the timeline of events to be able to get the tty alias
> working is the following:
>
> 1. The "set" command is registered as a prefix command in printcmd.c, which
> adds it to the global command list (cmdlist). setlist is the list of its
> subcommands.
> 2. The "set inferior-tty" command is added as a child of "set" (i.e. it's
> inserted into setlist) in infcmd.c.
> 3. The "tty" alias is created for "set inferior-tty" in infcmd.c. This
> looks up "set" in cmdlist, then "inferior-tty" in the subcommands of "set".
> It's found and everyone is happy.
>
> With the _initialize functions called in a different order, steps are
> executed in the order 2-3-1. When we try to create the alias, the "set"
> command has not been created and is therefore not part of cmdlist. We can't
> find the "set inferior-tty" command, so the alias is not created.
When we try to create the alias (in step 3), setlist is already created
in step 2, but it is not linked with cmdlist. The command "set
inferior-tty" is created in step 2 too. Since step 2 and step 3 are
close to each other (in _initialize_infcmd), we can pass the "set
inferior-tty" cmd_list_element to the place we create alias, like this,
cmd_name = "inferior-tty";
c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
gdb_assert (c != NULL);
add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
this makes sure we add alias "tty" under cmdlist, and it is alias to
"set inferior-tty". Step 1 just links setlist and cmdlist together.
>
> I think the core issue is that it's currently possible to create subcommands
> for prefixes that have not been created yet. We need some way of ensuring
> some kind of ordering. Here are some ideas:
>
How is the patch below? I run regress tests yet, and it still needs
some comments.
Comments
On 2017-05-16 16:49, Yao Qi wrote:
> On 17-05-16 09:51:42, Simon Marchi wrote:
>>
>> So indeed, the problem is due to the ordering of the _initialize
>> functions
>> that changed, _initialize_printcmd used to be called before
>> _initialize_infcmd, now it's the reverse.
>>
>> When it works, the timeline of events to be able to get the tty alias
>> working is the following:
>>
>> 1. The "set" command is registered as a prefix command in printcmd.c,
>> which
>> adds it to the global command list (cmdlist). setlist is the list of
>> its
>> subcommands.
>> 2. The "set inferior-tty" command is added as a child of "set" (i.e.
>> it's
>> inserted into setlist) in infcmd.c.
>> 3. The "tty" alias is created for "set inferior-tty" in infcmd.c.
>> This
>> looks up "set" in cmdlist, then "inferior-tty" in the subcommands of
>> "set".
>> It's found and everyone is happy.
>>
>> With the _initialize functions called in a different order, steps are
>> executed in the order 2-3-1. When we try to create the alias, the
>> "set"
>> command has not been created and is therefore not part of cmdlist. We
>> can't
>> find the "set inferior-tty" command, so the alias is not created.
>
> When we try to create the alias (in step 3), setlist is already created
> in step 2, but it is not linked with cmdlist. The command "set
> inferior-tty" is created in step 2 too. Since step 2 and step 3 are
> close to each other (in _initialize_infcmd), we can pass the "set
> inferior-tty" cmd_list_element to the place we create alias, like this,
>
> cmd_name = "inferior-tty";
> c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
> gdb_assert (c != NULL);
> add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
>
> this makes sure we add alias "tty" under cmdlist, and it is alias to
> "set inferior-tty". Step 1 just links setlist and cmdlist together.
>
>>
>> I think the core issue is that it's currently possible to create
>> subcommands
>> for prefixes that have not been created yet. We need some way of
>> ensuring
>> some kind of ordering. Here are some ideas:
>>
>
> How is the patch below? I run regress tests yet, and it still needs
> some comments.
It's very clean, I like it. I think it's good for master and 8.0. Do
you want to finish it or do you want me to pick it up?
I have enhanced gdb.base/set-inferior-tty.exp to exercise the alias, it
could be added to your patch:
https://github.com/simark/binutils-gdb/commit/1a8bcb5dbc0dde3d8a4d3f73d0a057b3831902dd#diff-2f94d545bf202185ae6a6022ebfa93bc
Thanks,
Simon
On 17-05-16 17:22:21, Simon Marchi wrote:
> It's very clean, I like it. I think it's good for master and 8.0. Do you
> want to finish it or do you want me to pick it up?
>
> I have enhanced gdb.base/set-inferior-tty.exp to exercise the alias, it
> could be added to your patch:
>
> https://github.com/simark/binutils-gdb/commit/1a8bcb5dbc0dde3d8a4d3f73d0a057b3831902dd#diff-2f94d545bf202185ae6a6022ebfa93bc
>
I'll finish it tomorrow.
@@ -284,16 +284,10 @@ deprecate_cmd (struct cmd_list_element *cmd, const char *replacement)
}
struct cmd_list_element *
-add_alias_cmd (const char *name, const char *oldname, enum command_class theclass,
- int abbrev_flag, struct cmd_list_element **list)
+add_alias_cmd (const char *name, cmd_list_element *old,
+ enum command_class theclass, int abbrev_flag,
+ struct cmd_list_element **list)
{
- const char *tmp;
- struct cmd_list_element *old;
- struct cmd_list_element *c;
-
- tmp = oldname;
- old = lookup_cmd (&tmp, *list, "", 1, 1);
-
if (old == 0)
{
struct cmd_list_element *prehook, *prehookee, *posthook, *posthookee;
@@ -307,7 +301,7 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
return 0;
}
- c = add_cmd (name, theclass, NULL, old->doc, list);
+ struct cmd_list_element *c = add_cmd (name, theclass, NULL, old->doc, list);
/* If OLD->DOC can be freed, we should make another copy. */
if (old->doc_allocated)
@@ -330,6 +324,21 @@ add_alias_cmd (const char *name, const char *oldname, enum command_class theclas
return c;
}
+struct cmd_list_element *
+add_alias_cmd (const char *name, const char *oldname, enum command_class theclass,
+ int abbrev_flag, struct cmd_list_element **list)
+{
+ const char *tmp;
+ struct cmd_list_element *old;
+ struct cmd_list_element *c;
+
+ tmp = oldname;
+ old = lookup_cmd (&tmp, *list, "", 1, 1);
+
+ return add_alias_cmd (name, old, theclass, abbrev_flag, list);
+}
+
+
/* Like add_cmd but adds an element for a command prefix: a name that
should be followed by a subcommand to be looked up in another
command list. PREFIXLIST should be the address of the variable
@@ -141,6 +141,12 @@ extern struct cmd_list_element *add_alias_cmd (const char *, const char *,
enum command_class, int,
struct cmd_list_element **);
+extern struct cmd_list_element *add_alias_cmd (const char *,
+ cmd_list_element *,
+ enum command_class, int,
+ struct cmd_list_element **);
+
+
extern struct cmd_list_element *add_prefix_cmd (const char *, enum command_class,
cmd_cfunc_ftype *fun,
const char *,
@@ -3210,7 +3210,10 @@ is restored."),
set_inferior_tty_command,
show_inferior_tty_command,
&setlist, &showlist);
- add_com_alias ("tty", "set inferior-tty", class_alias, 0);
+ cmd_name = "inferior-tty";
+ c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
+ gdb_assert (c != NULL);
+ add_alias_cmd ("tty", c, class_alias, 0, &cmdlist);
cmd_name = "args";
add_setshow_string_noescape_cmd (cmd_name, class_run,