Re: [PATCH] C++ patches

[Andrew Cagney <ac131313 at cygnus dot com>]

Kevin Buettner wrote:
> 
> On Sep 6,  9:35pm, Daniel Berlin wrote:
> 
> > I've rerun indent on the files this affects, because the formatting has
> > gotten way out of whack with the GNU standards.
> > I hope nobody minds, if somebody does, i'll undo the whitespace changes in
> > my patch
> > (diff -c3pwBb, remove the unbreaking of lines it did).
> 
> I think the generally agreed upon practice is to do two separate
> commits; the first being the actual changes you made and the second
> being the reformatting.  This way it's easier to review the first
> patch.

Either order.  The main thing is to keep indentation and real codeing
changes separate.  A maintainer shouldn't be presented with a patch that
contains a combination of both.  Reviewing it is impossible :-(.

Don't forget to clean the diff up a little (strip out the ChangeLog etc)
next time :-)

> > Please note we now have one memory leak in lookup_symbol. The demangler
> > gives us a buffer we never free. I'd need to free it at every exit point
> > from the function, of which we have 12 right now, all of which end almost
> > the exact same way (most return fixup_symbol_section(sym), a few just
> > return sym. Is this incorrect?).
> >
> > I'm thinking that i'll change it so we just have a label at the end, and
> > 12 gotos, rather than 12 exit points, and do the cleanup of the
> > buffer there.
> > It would make it about 50x easier for someone who needed to do something
> > that requires cleanup when we exit.
> 
> I think gotos in this case are preferable.

Gotos are bad.  M'kay?  We've so far spent three years trying to
eliminate all the goto's in WFI, adding more will just make me depressed
:-)

Why not push the ALL_SYMTABS() loop into a sub function parameterized by
``name'' - I'm assuming that it is the loop that has the N different
exit points?  That way you get your goto without actually having a
goto...  Hmm, I suspect that ``name'' will need a cleanup.  Throw an
error and, regardless of the gotos, it will leak memory.

Keep it up.

	Andrew