From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103291 invoked by alias); 16 May 2017 13:51:45 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 101948 invoked by uid 89); 16 May 2017 13:51:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=HX-PHP-Originating-Script:rcube.php, tonight, H*u:1.2.5, H*UA:1.2.5 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 16 May 2017 13:51:41 +0000 Received: by simark.ca (Postfix, from userid 33) id 889FE1E4B9; Tue, 16 May 2017 09:51:42 -0400 (EDT) To: Joel Brobecker Subject: Re: GDB 7.99.91 available for testing X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 16 May 2017 13:51:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: References: <20170504194442.63AAF60B72@joel.gnat.com> Message-ID: <1f06c802305c02d92e0806b6174d7963@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00346.txt.bz2 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: > > b593ecca856860a8b38deb808493bba4beef3aee > 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. > > Thanks, > > Simon Hi, 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/Makefile.in, 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? Simon