This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ia64: unwind directive handling


On Mon, 2005-06-27 at 03:32, Jan Beulich wrote:
> Here's the updated patch, with (I believe) all issues addressed (except for where I gave reasons for why things were done the way they are in an earlier reply). Additionally to that there are four more new warnings (noting inconsistencies between .prologue and later .save-s).

Sorry about the delays.  I'm hoping to be more responsive now that the
GCC Summit is over.  It messed up my schedule the last few weeks.

I have no problem with any of the answers you've given, or the
subsequent changes you made.

About the default in the middle of the switch statement issue, I missed
the fact that there was a missing break.  There is a relatively common
convention to add a /* FALLTHOUGH */ marker in this case.  This makes it
obvious that the missing break is intentional, and not a bug.  It also
makes it obvious that the case labels can't be reordered without
breaking the code.  Anyways, in this case, I think the rewrite is
better.  Your original code was too clever for its own good.  This
invariably leads to problems, as at some point in the future someone you
don't know will fail to understand how clever the code is, assume it is
broken, and then proceed to fix it.  Clear code is better than clever
code, even if it is a few lines longer.  And yes, gotos should be
avoided.

I said I was going to do some testing with the first version of the
patch.  glibc built OK.  I found two minor typos in the linux kernel,
spurious characters at the end of an unwind directive that were now
caught whereas they used to be ignored.  Both have been reported, and I
think both have already been fixed.  In any case, there is nothing to
worry about here.  I may have forgetten to do the gcc bootstrap.  I
don't recall.

The patch is OK.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]