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

[Fernando Nasser <fnasser at cygnus dot com>]

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?
> 

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.

Fernando



> Michael
> 
> >
> > Ok?
> >         Andrew
> >
> >     ---------------------------------------------------------------
> > Mon Nov 27 18:19:46 2000  Andrew Cagney  <cagney@b1.cygnus.com>
> >
> >         * symfile.c (add_symbol_file_command): Always initialize
> >         my_cleanup using a NULL cleanup.
> >
> > Index: symfile.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/symfile.c,v
> > retrieving revision 1.20
> > diff -p -r1.20 symfile.c
> > *** symfile.c   2000/10/27 15:02:42     1.20
> > --- symfile.c   2000/11/27 07:22:03
> > *************** add_symbol_file_command (char *args, int
> > *** 1416,1422 ****
> >     } sect_opts[SECT_OFF_MAX];
> >
> >     struct section_addr_info section_addrs;
> > !   struct cleanup *my_cleanups;
> >
> >     dont_repeat ();
> >
> > --- 1417,1423 ----
> >     } sect_opts[SECT_OFF_MAX];
> >
> >     struct section_addr_info section_addrs;
> > !   struct cleanup *my_cleanups = make_cleanup (null_cleanup, NULL);
> >
> >     dont_repeat ();
> >
> > *************** add_symbol_file_command (char *args, int
> > *** 1452,1458 ****
> >         {
> >           /* The first argument is the file name. */
> >           filename = tilde_expand (arg);
> > !         my_cleanups = make_cleanup (free, filename);
> >         }
> >         else
> >         if (argcnt == 1)
> > --- 1453,1459 ----
> >         {
> >           /* The first argument is the file name. */
> >           filename = tilde_expand (arg);
> > !         make_cleanup (free, filename);
> >         }
> >         else
> >         if (argcnt == 1)

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9