This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: GDB 7.99.91 available for testing

On 2017-05-15 17:11, Simon Marchi wrote:
Hi Joel,

Somebody reported on IRC that the command "tty" (an alias for "set
inferior-tty") didn't work anymore.  git bisect found one of my
commits as the culprit:

  Makefiles: Flatten and sort file lists

This is likely due to the _initialize functions being called in a
different order.  I think this should block the 8.0 release.  I'll
have time to investigate this tonight or tomorrow.




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.

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:

1. Force some kind of ordering through init.c

It's already done for at least one file, if you look at gdb/, you'll see that the gdbtypes file is put at the beginning, so that it's called first. We could do the same with printcmd. This is not a solution I would like in the long term, but maybe it's good for the 8.0 release, as it would minimize the required changes.

2. Add dependencies between _initialize_functions

We could have a way for _initialize functions to call each other. For example, since we know that _initialize_printcmd must be called before _initialize_infcmd, the latter could call the former. A static flag in each function could ensure that each is called only once.

I don't like this one very much, since the dependencies between functions have to be manually deduced, they can become outdated and it's easy to forget some.

3. Create prefix commands on demand

Right now, if you want to create a prefix command under "set", you just refer to &setlist, regardless of whether the "set" command has been created or not yet. Instead of referring directly to setlist, we could call something like set_prefix_list, which would create the "set" command if it hasn't been created yet, and return setlist. Something like that:

  cmd_list_element **set_prefix_list ()
    static cmd_list_element *setlist = NULL;
    bool initialized = false;

    if (!initialized)
        // Create the set command
        add_prefix_cmd ("set", ..., &setlist, ...);

    return &setlist;

References to &setlist would be changed to set_prefix_list (). This encodes the dependencies directly in the code: if want to add a subcommand to "set", you need to call set_prefix_list to obtain the list of "set"'s children, ensuring that "set" has been created. So the deps can't get stale, or you can't miss one by mistake.

So far this is the approach I like the most for the master branch. There are many ways the code could be improved further (C++, better encapsulation), but this would be a manageable first step. But for the immediate needs of the imminent 8.0 release, I think #1 would be better.

Any comments?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]