From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 119495 invoked by alias); 25 Jan 2017 18:11:18 -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 100371 invoked by uid 89); 25 Jan 2017 18:11:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=assessment, subsystem, H*i:sk:aa7d56e, H*MI:sk:aa7d56e X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 25 Jan 2017 18:11:07 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cWS2E-0001DD-5M from Luis_Gustavo@mentor.com ; Wed, 25 Jan 2017 10:11:06 -0800 Received: from [172.30.8.148] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Wed, 25 Jan 2017 10:11:03 -0800 Reply-To: Luis Machado Subject: Re: [PATCH/RFC] Refactor gdb.reverse/insn-reverse.c References: <1484593854-1791-1-git-send-email-lgustavo@codesourcery.com> <20170125161011.GW28060@E107787-LIN> To: Pedro Alves , Yao Qi CC: From: Luis Machado Message-ID: <9b4f74b9-b537-7513-2346-817d03075e76@codesourcery.com> Date: Wed, 25 Jan 2017 18:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-02.mgc.mentorg.com (147.34.90.202) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2017-01/txt/msg00547.txt.bz2 On 01/25/2017 11:44 AM, Pedro Alves wrote: > On 01/25/2017 04:50 PM, Luis Machado wrote: >> On 01/25/2017 10:10 AM, Yao Qi wrote: >>> On 17-01-16 13:10:54, Luis Machado wrote: >>>> I've broken up the main file into other files with arch-specific bits >>>> (insn-support-.c). The main file will hold the generic pieces >>>> that will >>>> take care of calling the tests. >>>> >>> >>> It is reasonable to me. Can we name arch-specific files as >>> insn-reverse-.c? >>> >> >> Thanks for the review. >> >> Would you reconsider this? I named it insn-support-.c because we >> already know this is a reverse debugging test (from gdb.reverse) and we >> are really testing instruction support. I'm fine either way though, and >> just wanted to add a little bit more context in the name. > > My 2c nit. > > "support" doesn't sound ideal to me. By that logic, > every test in the testsuite is testing gdb's support of some > feature, so "support" is redundant? > > When I see a file named "support", I think it's some base/foundation > (==support) framework. Is that the case here? I had understood > that the "base" is in insn-reverse.c instead? Or are the > insn-support-.c files meant to be shared between multiple > test cases, thus they'll be providing "support" for a multitude > of random tests? insn-reverse-.c at least gave a hint > that the files are all related to insn-reverse.c. > > Also, if "reverse" is redundant in "insn-reverse-.c", then > it is also redundant in "insn-reverse.c" too, so you should be arguing > for renaming "insn-reverse.c|exp" -> "insn-support.c|exp" too, which > would preserve the similarity of the names between the driver > file + the arch files. That is a reasonable assessment. insn-reverse.[c|exp] is redundant and IMO would benefit from renaming too. The support in "insn-support-.c means support for a set of instructions for this particular subsystem of gdb, therefore why i went with that name. Thinking about it further, instruction decoding support is the basis/foundation of reverse debugging, without which things would not work properly. But i may be overthinking. :-) I didn't analyze it in depth though. It just seemed to me the naming was slightly better with less redundancy. But, like i said, i'm fine either way.