Re: [PATCH] [SH] Prologue skipping if there is none

[Pedro Alves <palves@redhat.com>]

On 03/02/2012 11:17 AM, Thomas Schwinge wrote:
> On Thu, 1 Mar 2012 17:18:47 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
>> > On Thu, 01 Mar 2012 10:00:00 +0100
>> > Thomas Schwinge <thomas@codesourcery.com> 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