From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 79056 invoked by alias); 16 Feb 2016 15:26:30 -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 79038 invoked by uid 89); 16 Feb 2016 15:26:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:1454, writer, shortcuts, displaced X-HELO: usplmg21.ericsson.net Received: from usplmg21.ericsson.net (HELO usplmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 16 Feb 2016 15:26:28 +0000 Received: from EUSAAHC001.ericsson.se (Unknown_Domain [147.117.188.75]) by usplmg21.ericsson.net (Symantec Mail Security) with SMTP id C4.DD.32102.A8F33C65; Tue, 16 Feb 2016 16:26:02 +0100 (CET) Received: from [142.133.182.13] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.77) with Microsoft SMTP Server id 14.3.248.2; Tue, 16 Feb 2016 10:26:25 -0500 Subject: Re: [PATCH 0/3] Minor refactorings in arm-tdep.c instruction decoding To: Yao Qi References: <1455121027-27061-1-git-send-email-simon.marchi@ericsson.com> <86si0zpkbl.fsf@gmail.com> CC: From: Simon Marchi Message-ID: <56C33FA1.7010409@ericsson.com> Date: Tue, 16 Feb 2016 15:26:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <86si0zpkbl.fsf@gmail.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00481.txt.bz2 On 16-02-11 06:17 AM, Yao Qi wrote: > Simon Marchi writes: > >> I am currently working on extracting the instruction decoding from the >> displaced stepping support in arm-tdep.c, in order to share the functionality >> with the upcoming fast tracepoint support. I did a few refactors that helped >> me correlate the code with the ARM Architecture Reference Manual. I think the >> change helps readability in general, and especially when you have the manual >> open on the side. >> >> The idea is to follow the the order of the manual, use the same names and do >> the same "checks" (avoid using unnecessary shortcuts that make the code more >> cryptic). > > It is good to match the code to the manual. The instruction encoding > can't change, but the order or the category of instruction _may_ change in > the manual in the future. I am not the manual writer, but writers may > want to refactor the doc in the future too :) > > Although your change is code refactor, better to run tests. Yes, that is the part that is worrying me. Since we don't have unit tests, I am too afraid to break things. I'll forget this patch set for the time being. When the code is extracted to its own file and decoupled from the core of GDB, maybe there will be some way to write some kind of unit tests that ensure that the refactor is safe. Thanks for reviewing anyway!