From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21338 invoked by alias); 2 Mar 2012 12:01:04 -0000 Received: (qmail 21326 invoked by uid 22791); 2 Mar 2012 12:01:03 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 02 Mar 2012 12:00:48 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q22C0cY0014430 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 2 Mar 2012 07:00:43 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q22C0bAR028862; Fri, 2 Mar 2012 07:00:37 -0500 Message-ID: <4F50B664.3090307@redhat.com> Date: Fri, 02 Mar 2012 12:01:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 MIME-Version: 1.0 To: Thomas Schwinge CC: Kevin Buettner , gdb-patches@sourceware.org Subject: Re: [PATCH] [SH] Prologue skipping if there is none References: <87pqdgciho.fsf@schwinge.name> <20120215075413.1313f7fa@mesquite.lan> <20120215165907.33f2e9a6@mesquite.lan> <8739aad9il.fsf@schwinge.name> <20120216182544.36b41a1b@mesquite.lan> <87mx8da3b9.fsf@schwinge.name> <20120220162029.2082b6a1@mesquite.lan> <87wr7c7aop.fsf@schwinge.name> <20120224144657.36bbd09f@mesquite.lan> <87r4xd528y.fsf@schwinge.name> <87d38w4rxr.fsf@schwinge.name> <20120301171847.306829ba@mesquite.lan> <87mx7z2qwc.fsf@schwinge.name> In-Reply-To: <87mx7z2qwc.fsf@schwinge.name> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-03/txt/msg00080.txt.bz2 On 03/02/2012 11:17 AM, Thomas Schwinge wrote: > On Thu, 1 Mar 2012 17:18:47 -0700, Kevin Buettner wrote: >> > On Thu, 01 Mar 2012 10:00:00 +0100 >> > Thomas Schwinge wrote: >> > >>> > > @@ -594,6 +590,7 @@ sh_analyze_prologue (struct gdbarch *gdb >>> > > { >>> > > sav_reg = reg; >>> > > offset = (inst & 0xff) << 1; >>> > > + /* TODO: check that this is a valid address. */ >>> > > sav_offset = >>> > > read_memory_integer ((pc + 4) + offset, 2, byte_order); >>> > > } >> > >> > FIXME and TODO comments are generally frowned upon. All too often, >> > they end up hanging about for many years. >> > You may want to just keep that TODO comment in your tree or in >> > some other TODO list on the side. > Hmm, I don't agree. I think it's better to have such comments in a > central place, instead of each developer having their own set of them in > their own files. I do agree that source code comments are not useful for > more *general* ``work to be done'', but this is a very local issue, where > the comment applies directly to the next line. Anyway, I'm not the one > to set the rules here; I've taken these out. FWIW, I agree that FIXMEs and TODOs in the code can be helpful. I've learnt immensely about gdb's intended direction from the FIXME's in place. However, there should be a high barrier to adding new FIXMEs. If the fix is known, and it doesn't involve e.g., deep design level changes, then we should just get it fixed before the change lands in the tree. In this case, what's necessary to just fix that particular issue? -- Pedro Alves