This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfa] symfile.c - fix apparently uninitialized my_cleanups
- To: Fernando Nasser <fnasser at cygnus dot com>
- Subject: Re: [rfa] symfile.c - fix apparently uninitialized my_cleanups
- From: Andrew Cagney <ac131313 at cygnus dot com>
- Date: Wed, 29 Nov 2000 12:50:46 +1100
- Cc: Michael Snyder <msnyder at redhat dot com>,GDB Patches <gdb-patches at sourceware dot cygnus dot com>
- References: <3A220AD1.83623AC9@cygnus.com> <3A2284AD.22D4@redhat.com> <3A228ABD.10472E6@cygnus.com>
Fernando Nasser wrote:
>
> Michael Snyder wrote:
> >
> > Andrew Cagney wrote:
> > >
> > > Hello,
> > >
> > > I think the attatched is correct. ``my_cleanups'' was always
> > > initialized but in a somewhat obscure way. It probably illustrates a
> > > better way of doing cleanups for a function - unconditionally setup the
> > > cleanup at the very start.
> > Andrew,
> >
> > There's nothing wrong with your change, but since there are
> > probably hundreds of other functions that do things in the
> > same way this one did, do you want to say any more about
> > why it's bad? Maybe motivate a few people to follow your
> > example?
The last thing I want is for people to run around changing code that
isn't broken. Here it was only broken in that GCC -Wuninitialized gave
a warning.
I should have said ``simpler'' rather than ``better''. Both, in the
end, are pretty subjective.
Per Fernando's comments:
> I believe that, with the initialization being inside an "if", the
> compiler will issue a warning "my_cleanups may be used uninitialized...".
> It is always good to get silly warnings out of the way so they don't clobber
> the ones that really indicate something fishy.
>
> Also, as a question of style, what Andrew is proposing is safer. Although
> it is not necessary everywhere, it is good when cleanups are inside conditionals. If someone does
> forget a path of execution that does not set
> "my_cleanups" (or equivalent) and do call do_cleanups(my_cleanups) we may
> end up cleaning up too much stuff.
Yes. The code was changed to:
{
create dummy cleanup;
complicated code that just might
allocate memory and need a cleanup
do_cleanups()
}
Ihe simplification was to create a dummy cleanup whether it was needed
or not. That ment that the ``complicated code'' (actually it wasn't :-)
didn't, in addition to everything else, need to check to see if the
cleanup chain needed saving.
Andrew