This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


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

Re: [rfa] symfile.c - fix apparently uninitialized my_cleanups


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

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